[feature/bots] Automate assigning/unassigning issues #571#572
[feature/bots] Automate assigning/unassigning issues #571#572Eeshu-Yadav wants to merge 6 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:
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive GitHub automation system for issue and PR lifecycle management. It adds three specialized bots (IssueAssignmentBot, StalePRBot, PRReopenBot) built on a shared GitHubBot base class for GitHub API interaction. Four new GitHub Actions workflows trigger these bots in response to issue comments, PR events, and scheduled runs. The system automates assignment request responses with contributing guidelines, automatic issue assignment and unassignment based on PR links, stale PR detection with warnings and closures, and PR reopening with contributor activity handling. Complete test coverage and utility functions for parsing linked issues are included. Sequence Diagram(s)sequenceDiagram
actor User as Contributor
participant GH as GitHub
participant Bot as IssueAssignmentBot
participant API as GitHub API
User->>GH: Comments "please assign"
GH->>Bot: Triggers issue_comment event
Bot->>Bot: Detects assignment request
Bot->>API: Fetch contributing guidelines
API-->>Bot: Guidelines URL
Bot->>API: Post comment with guidelines
API-->>GH: Comment created
GH-->>User: Guidance displayed
User->>GH: Opens related PR
GH->>Bot: Triggers pull_request_target event
Bot->>Bot: Extract linked issues from PR body
Bot->>API: Fetch issue details
API-->>Bot: Issue data
Bot->>Bot: Validate issue state
Bot->>API: Assign issue to PR author
API-->>GH: Assignee updated
GH-->>User: Issue assigned to author
sequenceDiagram
participant Scheduler as Scheduled Trigger
participant Bot as StalePRBot
participant GH as GitHub API
participant PR as Pull Request
Scheduler->>Bot: Cron job triggers (daily)
Bot->>Bot: Initialize client
Bot->>GH: Fetch all open PRs
GH-->>Bot: List of open PRs
loop For each PR
Bot->>GH: Get last changes requested
GH-->>Bot: Review timestamp
Bot->>Bot: Calculate days since activity
alt Days > 60
Bot->>GH: Check if already closed
Bot->>GH: Post closing comment
Bot->>GH: Close PR
Bot->>GH: Unassign linked issues
GH-->>PR: PR closed, issues unassigned
else Days > 14
Bot->>GH: Check for stale marker
Bot->>GH: Mark PR stale
Bot->>GH: Add stale label
Bot->>GH: Unassign linked issues
GH-->>PR: Stale label added
else Days > 7
Bot->>GH: Post warning comment
GH-->>PR: Warning comment visible
end
end
sequenceDiagram
participant GH as GitHub
participant Bot as PRReopenBot
participant API as GitHub API
participant PR as Pull Request
GH->>Bot: Pull request reopened event
Bot->>Bot: Load event payload
Bot->>Bot: Extract PR details and author
Bot->>Bot: Parse linked issues from PR body
loop For each linked issue
Bot->>API: Fetch issue details
API-->>Bot: Issue data with assignees
Bot->>Bot: Check if unassigned or author-assigned
Bot->>API: Assign issue to PR author
Bot->>API: Post welcome comment
API-->>PR: Issue assigned, comment added
end
Bot->>API: Remove stale label from PR
API-->>GH: PR updated with reassignments
GH-->>PR: Issues reassigned to author
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
| const prBody = pr.body || ''; | ||
| const issuePattern = /(?:fix(?:es)?|close[sd]?|resolve[sd]?)\s+#(\d+)/gi; | ||
| const matches = [...prBody.matchAll(issuePattern)]; | ||
| for (const match of matches) { | ||
| const issueNumber = parseInt(match[1], 10); | ||
| try { | ||
| await github.rest.issues.removeAssignees({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| assignees: [prAuthor] | ||
| }); | ||
| console.log(`Unassigned ${prAuthor} from issue #${issueNumber}`); |
There was a problem hiding this comment.
When extracting linked issues from the PR body, duplicates aren’t deduplicated and PR references aren’t filtered out. This can lead to repeated API calls (and possible rate limiting) and may attempt to unassign from referenced pull requests. Consider tracking processedIssues (as done in other workflows) and fetching the item to skip issue.data.pull_request before unassigning.
|
|
||
| permissions: | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
The workflow is granting pull-requests: write, but it only reads PR data (and modifies issues/labels). Reducing this to pull-requests: read limits token scope and avoids unnecessary write privileges.
| pull-requests: write | |
| pull-requests: read |
| name: 'stale' | ||
| }); | ||
| } catch (e) { | ||
|
|
There was a problem hiding this comment.
This empty catch block swallows failures when removing the stale label, which makes debugging harder (e.g., permission changes, API errors). Consider at least logging the error or explicitly handling the expected 404 case.
| const status = e?.status ?? e?.response?.status; | |
| if (status === 404) { | |
| console.log(`'stale' label not found on PR #${prNumber}, nothing to remove.`); | |
| } else { | |
| console.error(`Failed to remove 'stale' label from PR #${prNumber}:`, e); | |
| } |
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const contributingUrl = 'http://openwisp.io/docs/developer/contributing.html'; |
There was a problem hiding this comment.
The contributing URL is using plain HTTP. Prefer HTTPS to avoid mixed-content/security issues and ensure users are sent to the secure version of the docs.
| const contributingUrl = 'http://openwisp.io/docs/developer/contributing.html'; | |
| const contributingUrl = 'https://openwisp.io/docs/developer/contributing.html'; |
| const { data: pullRequests } = await github.rest.pulls.list({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| state: 'open', | ||
| per_page: 100 | ||
| }); |
There was a problem hiding this comment.
The workflow only fetches the first page of open PRs (per_page: 100) and does not paginate, so repos with >100 open PRs will silently skip the rest. Consider using github.paginate(...) (or manual page looping) to process all open PRs reliably.
| const { data: pullRequests } = await github.rest.pulls.list({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| per_page: 100 | |
| }); | |
| const pullRequests = await github.paginate( | |
| github.rest.pulls.list, | |
| { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| per_page: 100 | |
| } | |
| ); |
| 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.
| call_args = mock_issue.create_comment.call_args[0][0] | ||
| self.assertIn("@testuser", call_args) | ||
| self.assertIn("contributing guidelines", call_args) | ||
| # Generic format includes both Fixes and Closes | ||
| self.assertTrue("Fixes `#123`" in call_args or "Closes `#123`" in call_args) |
There was a problem hiding this comment.
Misleading test comment: this actually hits the bug-specific path, not the generic format.
The mock has title="Test issue title", which contains the word "issue" — a bug keyword. So detect_issue_type returns "bug" and the message uses `Fixes `#123 ``, not the generic format. The comment on line 112 ("Generic format includes both Fixes and Closes") is inaccurate.
Fix the comment or the test data
Either update the comment:
- # Generic format includes both Fixes and Closes
- self.assertTrue("Fixes `#123`" in call_args or "Closes `#123`" in call_args)
+ # "issue" in title triggers bug detection → Fixes keyword
+ self.assertIn("Fixes `#123`", call_args)Or use a title/body without bug/feature keywords to test the generic path:
- mock_issue.title = "Test issue title"
- mock_issue.body = "Test issue body"
+ mock_issue.title = "Some task"
+ mock_issue.body = "Description of task"🤖 Prompt for AI Agents
In `@openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py` around lines
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.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
374ace8 to
2315ec8
Compare
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
a03de86 to
771fb36
Compare
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
See my comments below.
@coderabbitai please review and focus on security, bugs, performance.
| REPOSITORY: ${{ github.repository }} | ||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||
| run: > | ||
| python .github/actions/bot-autoassign/__main__.py |
There was a problem hiding this comment.
Will this be available in other repositories?
There was a problem hiding this comment.
The way it is currently written in the docs (run: python .github/actions/bot-autoassign/__main__.py), it strictly relies on the repository checking out openwisp-utils to access the bot scripts.
To make this cleanly available across all OpenWISP repositories, I think we have two solid options:
Option A: The "bot-testing-ground" approach
Other repositories can just copy the workflow YAML directly, but they must explicitly add a step to checkout openwisp-utils right before running the Python script, exactly like we tested in bot-testing-ground originally.
- name: Checkout openwisp-utils for bot scripts
uses: actions/checkout@v4
with:
repository: openwisp/openwisp-utils
path: bot-scripts
- name: Install PyGithub
run: pip install PyGithub
- name: Run script
run: python bot-scripts/.github/actions/bot-autoassign/__main__.pyOption B: Reusable Workflows
We convert these 4 workflow YAML files into Reusable Workflows (just like the bot-ci-failure bot). This way, other repositories only need a tiny "caller" YAML workflow that simply points to openwisp/openwisp-utils/.github/workflows/bot-autoassign-*.yml@master. They wouldn't need to worry about explicitly checking out scripts or installing PyGithub.
I used Option A in the testing ground and planning to use the similar approach in other repos.
|
|
||
| def get_contributing_guidelines_url(self): | ||
| """Get the contributing guidelines URL for the repository""" | ||
| return "https://openwisp.io/docs/developer/contributing.html" |
There was a problem hiding this comment.
| return "https://openwisp.io/docs/developer/contributing.html" | |
| return "https://openwisp.io/docs/stable/developer/contributing.html" |
|
|
||
| if issue.pull_request: | ||
| continue | ||
|
|
There was a problem hiding this comment.
Too many blank lines, really, too many. Get rid of most blank lines please. Use them sparingly or tell your AI generator so.
| "We understand that life gets busy, and we appreciate " | ||
| "your initial contribution! 💙", | ||
| "", | ||
| "**The door is always open** for you to come back:", |
There was a problem hiding this comment.
See openwisp/bot-testing-ground#71 (comment)
Please keep the same in mind for the other bots. Let's not add unnecessary line breaks in the bot comments.
| @@ -0,0 +1,278 @@ | |||
| import os | |||
There was a problem hiding this comment.
@Eeshu-Yadav how are these tests run in the CI BTW? The runtests.py script needs to be updated. Make sure tests can be run by runtests.py.
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
771fb36 to
85fdb4e
Compare
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
85fdb4e to
c187836
Compare
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
c187836 to
574cbbb
Compare
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
574cbbb to
9263223
Compare
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
9263223 to
3750bbd
Compare
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
3750bbd to
3226379
Compare
Created four GitHub Actions workflow files to automate issue assignment/unassignment: 1. issue-assignment.yml - Auto-response to 'assign me' requests 2. pr-issue-link.yml - Auto-assign on PR 3. stale-pr-management.yml - Stale PR management (tracks contributor inactivity only) 4. pr-reopen-reassign.yml - Handle contributor follow-up Fixes openwisp#571
- Included review comments & reviews in activity checks. - Cached comments & optimized pagination calls to prevent double fetching. - Swapped close API call before posting the closure comment. - Corrected workflow unassign event via action == closed. - Updated extract mapping to support 'fixed' keyword. - Upper bounded PyGithub version. - Removed unnecessary shebangs from bot modules. Fixes openwisp#572
…p#571 Added concurrency groups to all 4 bot-autoassign workflow files to prevent multiple concurrent runs for the same issue/PR. Also updated the docs YAML example to include concurrency. Closes openwisp#571
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
3226379 to
7c15479
Compare
- Remove needless blank lines in __main__.py (between import and return) - Fix contributing guidelines URL to include /stable/ path segment - Reformat bot comment strings to avoid unnecessary line breaks in rendered markdown while staying within 110 char limit Closes openwisp#571
CI Failure: Test Failures and Style IssuesHello @nemesifier and @Eeshu-Yadav, (Analysis for commit cf77fc2) It looks like there are a few issues that need to be addressed in your commit. Failure 1: Code Style (flake8)Explanation: Remediation: openwisp-qa-formatThis command will automatically format your code according to the project's style guidelines, resolving most |
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
cf77fc2 to
8f1b67b
Compare
There was a problem hiding this comment.
Great progress @Eeshu-Yadav I think we're almost there, please see my comments below and address/reply to all of them as I am still not convinced about some details but I may also be wrong so I want to hear your opinion.
| class TestExtractLinkedIssues: | ||
| @pytest.mark.parametrize( | ||
| "pr_body,expected", | ||
| [ | ||
| ("Fixes #123", [123]), | ||
| ("Closes #456 and resolves #789", [456, 789]), | ||
| ("fix #100, close #200, resolve #300", [100, 200, 300]), | ||
| ("This PR fixes #123 and closes #123", [123]), # dedup | ||
| ("Fixes: #42", [42]), # colon syntax | ||
| ("Related to #99", [99]), # relates-to | ||
| ("Fixes owner/repo#55", [55]), # cross-repo | ||
| ("Fixed #999", [999]), | ||
| ("No issue references here", []), | ||
| ("", []), | ||
| (None, []), | ||
| ], | ||
| ) | ||
| def test_extract_linked_issues(self, pr_body, expected, bot_env): | ||
| from .utils import extract_linked_issues | ||
|
|
||
| result = extract_linked_issues(pr_body) | ||
| assert sorted(result) == sorted(expected) |
There was a problem hiding this comment.
@Eeshu-Yadav please address this, I see it has a point. Is this real duplication? Or is just different tests having the same name (which means the name should be made slightly different)?
| class TestPRReopenBot: | ||
| @pytest.fixture(autouse=True) | ||
| def setup(self, monkeypatch): | ||
| monkeypatch.setenv("GITHUB_TOKEN", "test_token") | ||
| monkeypatch.setenv("REPOSITORY", "openwisp/openwisp-utils") | ||
| monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request_target") | ||
|
|
||
| with patch("openwisp_utils.bots.auto_assign.base.Github") as mock_github_cls: | ||
| mock_repo = Mock() | ||
| mock_github_cls.return_value.get_repo.return_value = mock_repo | ||
| self.mock_github = mock_github_cls | ||
| self.mock_repo = mock_repo | ||
| yield |
There was a problem hiding this comment.
@Eeshu-Yadav is this difference necessary or can we achieve consistency?
| def detect_issue_type(self, issue): | ||
| """Intelligently detect issue type. | ||
|
|
||
| Analyzes labels, title and body and returns 'bug', 'feature', or | ||
| None. | ||
| """ | ||
| bug_keywords = [ | ||
| "bug", | ||
| "error", | ||
| "crash", | ||
| "fail", | ||
| "broken", | ||
| "problem", | ||
| "not working", | ||
| "doesn't work", | ||
| "does not work", | ||
| "fix", | ||
| "incorrect", | ||
| "wrong", | ||
| "exception", | ||
| "traceback", | ||
| "breaking", | ||
| "regression", | ||
| ] | ||
|
|
||
| feature_keywords = [ | ||
| "feature", | ||
| "enhancement", | ||
| "add", | ||
| "implement", | ||
| "support", | ||
| "new", | ||
| "create", | ||
| "allow", | ||
| "enable", | ||
| "improve", | ||
| "improvement", | ||
| "upgrade", | ||
| "extend", | ||
| "functionality", | ||
| "capability", | ||
| "ability", | ||
| "option", | ||
| ] | ||
|
|
||
| issue_labels = [label.name.lower() for label in issue.labels] | ||
| if any(label in issue_labels for label in ["bug", "bugfix", "fix"]): | ||
| return "bug" | ||
| elif any( | ||
| label in issue_labels for label in ["feature", "enhancement", "improvement"] | ||
| ): | ||
| return "feature" | ||
|
|
||
| title = (issue.title or "").lower() | ||
| body = (issue.body or "").lower() | ||
| combined_text = f"{title} {body}" | ||
|
|
||
| # Use word boundary matching to avoid false positives | ||
| bug_score = sum( | ||
| 1 | ||
| for keyword in bug_keywords | ||
| if re.search(rf"\b{re.escape(keyword)}\b", combined_text) | ||
| ) | ||
| feature_score = sum( | ||
| 1 | ||
| for keyword in feature_keywords | ||
| if re.search(rf"\b{re.escape(keyword)}\b", combined_text) | ||
| ) | ||
|
|
||
| if bug_score > feature_score and bug_score > 0: | ||
| return "bug" | ||
| elif feature_score > bug_score and feature_score > 0: | ||
| return "feature" | ||
|
|
||
| return None |
| def unassign_issues_from_pr(self, pr_body, pr_author): | ||
| """Unassign linked issues from PR author""" | ||
| if not self.repo: | ||
| print("GitHub client not initialized") | ||
| return [] | ||
|
|
||
| try: | ||
| linked_issues = extract_linked_issues(pr_body) | ||
| if not linked_issues: | ||
| return [] | ||
|
|
||
| unassigned_issues = [] | ||
|
|
||
| for issue_number in linked_issues: | ||
| try: | ||
| issue = self.repo.get_issue(issue_number) | ||
| if ( | ||
| hasattr(issue, "repository") | ||
| and issue.repository.full_name != self.repository_name | ||
| ): | ||
| print( | ||
| f"Issue #{issue_number} is from a different repository, skipping" | ||
| ) | ||
| continue | ||
| if issue.pull_request: | ||
| print(f"#{issue_number} is a PR, skipping unassignment") | ||
| continue | ||
|
|
||
| current_assignees = [ | ||
| assignee.login | ||
| for assignee in issue.assignees | ||
| if hasattr(assignee, "login") | ||
| ] | ||
| if pr_author in current_assignees: | ||
| issue.remove_from_assignees(pr_author) | ||
| unassigned_issues.append(issue_number) | ||
| print(f"Unassigned {pr_author} from issue #{issue_number}") | ||
|
|
||
| except Exception as e: | ||
| print(f"Error unassigning issue #{issue_number}: {e}") | ||
|
|
||
| return unassigned_issues | ||
|
|
||
| except Exception as e: | ||
| print(f"Error in unassign_issues_from_pr: {e}") | ||
| return [] |
| permissions: | ||
| issues: write | ||
| pull-requests: write |
| REPOSITORY: ${{ github.repository }} | ||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||
| run: > | ||
| python .github/actions/bot-autoassign/__main__.py |
|
@coderabbitai review, please focus on real issues: security, bugs, performance, unused code, wrong/outdated docs. |
|
Focus on real issues only: security vulnerabilities, correctness bugs, performance problems, unused/dead code, and wrong or outdated documentation. Skip style, formatting, and minor nit-level suggestions. ✅ Actions performedFull review triggered. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 16322 🏁 Script executed: Length of output: 703 🏁 Script executed: Length of output: 2428 🏁 Script executed: Length of output: 86 🏁 Script executed: Length of output: 81325
✅ Confirmed Fixed (from
|
| # | Bug | Status |
|---|---|---|
| 1 | Stale bot ignored inline review comments | ✅ Fixed — get_review_comments() is now called and iterated in stale_pr_bot.py |
| 2 | "Welcome back" comment always fired | ✅ Fixed — create_comment is now inside the if pr_author not in current_assignees: block |
| 3 | extract_linked_issues missed fixed |
✅ Fixed — pattern is now fix(?:e[sd])?, matching fix, fixes, fixed |
| 4 | unassign_issues_from_pr was dead code |
✅ Fixed — handle_pull_request now handles closed, and the workflow includes closed |
| 5/6 | Eager list materialization + redundant comment fetches | ✅ Fixed — get_pulls() no longer wrapped in list(), comments fetched once and passed through |
| 7 | PyGithub had no upper version bound |
✅ Fixed — now PyGithub>=2.0.0,<3.0.0 |
| 8 | Closure comment posted before PR was closed | ✅ Fixed — pr.edit(state="closed") now precedes pr.create_issue_comment(...) |
🔴 New Critical Bug
bot-autoassign-pr-reopen.yml — pull-requests: read must be pull-requests: write
File: .github/workflows/bot-autoassign-pr-reopen.yml
permissions:
contents: read
issues: write
pull-requests: read # ← BUG: must be writeBoth PRReopenBot.remove_stale_label() and PRActivityBot.handle_contributor_activity() call pr.remove_from_labels("stale"), which is a write operation on the PR. With only read permission this call will fail with HTTP 403 on every execution, silently breaking all stale-label removal. The bot-autoassign-stale-pr.yml correctly uses pull-requests: write — the same is needed here.
Fix:
permissions:
contents: read
issues: write
pull-requests: write🟠 High Design Bug
Workflows reference local Python scripts — reuse in other repos will always fail
Files: all four workflow YAML files + docs/developer/reusable-github-utils.rst
Every workflow runs the bot via:
run: python .github/actions/bot-autoassign/__main__.py issue_assignment "$GITHUB_EVENT_PATH"The docs instruct other repositories to "copy the workflow files". But when another repo copies only the YAML files, the checkout step checks out that repo, which will not contain .github/actions/bot-autoassign/__main__.py. The python ... command will fail immediately with No such file or directory.
The docs must either:
- Instruct users to also copy the entire
.github/actions/bot-autoassign/directory, or - Restructure the package to expose a proper console-script entry point (e.g.
bot-autoassigninstalled viapip install -e .[github_actions]) and invoke it as a command rather than a raw file path, or - Make the workflows reusable workflows (
workflow_call) hosted inopenwisp-utilsso other repos call them without needing local copies of the scripts.
🟡 Moderate Bug
process_stale_prs: open_prs.totalCount may be None
File: .github/actions/bot-autoassign/stale_pr_bot.py
open_prs = self.repo.get_pulls(state="open")
print(f"Found {open_prs.totalCount} open pull requests")PyGithub's PaginatedList.totalCount is populated only when GitHub's API response includes a total_count field. The GET /repos/{owner}/{repo}/pulls endpoint does not return this field, so open_prs.totalCount will be None. The log line will print "Found None open pull requests" on every run. Replace with a lazy count or remove it until the list has been iterated.
🟡 Moderate Bug
close_stale_pr: PR closed before comment — partial failure leaves PR silently closed
File: .github/actions/bot-autoassign/stale_pr_bot.py
pr.edit(state="closed") # PR is closed here
pr.create_issue_comment(...) # if this raises, PR is closed with no messageIf create_issue_comment raises (e.g. network error, rate limit), the PR is permanently closed with no closing comment and no unassignment of linked issues, with no retry or rollback. Consider wrapping the comment + unassignment in a try/finally or at minimum log a warning when the comment fails after a successful close.
🟡 Minor
GITHUB_EVENT_NAME not passed to stale PR bot workflow
File: .github/workflows/bot-autoassign-stale-pr.yml
- name: Run stale PR bot
env:
GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }}
REPOSITORY: ${{ github.repository }}
# GITHUB_EVENT_NAME missing (present in all other workflows)StalePRBot.run() does not branch on event_name, so this is not currently broken. However, self.event_name is None in the stale bot for no reason, inconsistent with the rest of the codebase and could silently break future changes.
📝 PR Title
The PR title [feature/bots] uses a scoped prefix that does not match the allowed pattern ^\[(feature|fix|change)\] in the changelog bot (.github/workflows/reusable-bot-changelog.yml). It should be renamed to [feature].
[full_review]
🧠 Learnings used
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
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.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 33-35: The top-level exception handler catching Exception as e
currently prints only e and loses the traceback; update the handler around the
block referencing bot_type to preserve full stack information by either calling
traceback.print_exc() or using logging.exception()/logger.exception() to log a
descriptive message including bot_type and the exception (e) so the traceback is
emitted to workflow logs—modify the except Exception as e: handler to emit the
traceback instead of just print and then return 1.
In @.github/actions/bot-autoassign/issue_assignment_bot.py:
- Around line 13-22: The current intent check uses raw substring matching
against assignment_phrases (variable assignment_phrases and the return any(...)
line) which causes false positives like "unassign me"; change the check to use
stricter token/regex matching—e.g., compile each phrase into a regex with word
boundaries (\b) or split the comment into tokens and match whole-word phrases,
and also add negative checks for common counterphrases like "unassign", "do not
assign", "don't assign" before returning true; update the any(...) logic to use
these word-boundary/negative filters so only true assignment intents trigger the
bot.
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 327-384: process_stale_prs currently swallows fatal exceptions and
run() returns True regardless; change process_stale_prs to return a boolean
status (True on success, False on fatal error) and ensure all exception paths
return False (e.g., the outer try/except should log and return False), then
update run() to call process_stale_prs and propagate its result (return False if
process_stale_prs returned False) so the action reflects failures instead of
always returning True; locate and modify the process_stale_prs and run functions
to implement this control flow change.
In @.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py:
- Around line 89-90: The test currently treats the string "Fixes owner/repo#55"
as producing a local issue ID 55 which removes repo context; update the
expectation for that test tuple to reflect local-only parsing (i.e., no local
IDs returned) so the test enforces safe behavior—change the tuple ("Fixes
owner/repo#55", [55]) to ("Fixes owner/repo#55", []) in the test data used by
test_issue_assignment_bot.py (the list of (message, expected_ids) cases).
In `@docs/developer/reusable-github-utils.rst`:
- Around line 77-80: The docs currently instruct copying only the workflow files
from .github/workflows (e.g., bot-autoassign-issue.yml,
bot-autoassign-pr-issue-link.yml, bot-autoassign-pr-reopen.yml,
bot-autoassign-stale-pr.yml) but later the workflow executes
.github/actions/bot-autoassign/__main__.py; update the documentation to require
copying the .github/actions/bot-autoassign/ directory (all bot scripts)
alongside the workflows or alternatively document a separate
“reusable-workflow-only” setup that removes references to
.github/actions/bot-autoassign/__main__.py so the workflow won’t fail at
runtime.
- Around line 69-73: The Secrets section incorrectly implies the bot runtime
consumes OPENWISP_BOT_APP_ID and OPENWISP_BOT_PRIVATE_KEY directly; update the
text to state that OPENWISP_BOT_APP_ID and OPENWISP_BOT_PRIVATE_KEY are required
only to mint a GITHUB_TOKEN, and that the bot runtime itself needs GITHUB_TOKEN
plus REPOSITORY and GITHUB_EVENT_NAME environment variables; adjust the prose
around the listed symbols (OPENWISP_BOT_APP_ID, OPENWISP_BOT_PRIVATE_KEY) and
add a short note mentioning the runtime requirements (GITHUB_TOKEN, REPOSITORY,
GITHUB_EVENT_NAME) so readers won’t confuse setup vs runtime secrets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f254762d-35df-4813-831b-d78484cc3a36
📒 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 (4)
📚 Learning: 2026-03-05T09:38:03.232Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
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/actions/bot-autoassign/utils.py.github/actions/bot-autoassign/pr_reopen_bot.py.github/workflows/bot-autoassign-stale-pr.yml.github/actions/bot-autoassign/stale_pr_bot.py.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py
📚 Learning: 2026-03-05T09:59:15.097Z
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:15.097Z
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/utils.py.github/actions/bot-autoassign/pr_reopen_bot.py.github/actions/bot-autoassign/tests/test_stale_pr_bot.py.github/actions/bot-autoassign/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
📚 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:
runtests.py
🔇 Additional comments (12)
runtests.py (1)
31-31: Added pytest target looks correct.Including
.github/actions/bot-autoassign/testshere ensures the new bot-autoassign test suite is exercised byruntests.py.docs/developer/reusable-github-utils.rst (1)
39-39: Good update toactions/checkout@v6.This keeps the example aligned with current action versions and consistency across the docs.
setup.py (1)
79-82: Good dependency boundary forPyGithubingithub_actionsextra.This version range is appropriately constrained for workflow stability.
.github/actions/bot-autoassign/utils.py (1)
4-18: Regex extraction logic looks solid.Nice job handling empty input, keyword variants, optional repo prefix, and stable deduplication order.
.github/workflows/bot-autoassign-stale-pr.yml (1)
8-15: Workflow wiring and permissions are well-scoped.The token generation, permission set, and stale bot execution path are correctly structured for scheduled automation.
Also applies to: 21-45
.github/workflows/bot-autoassign-issue.yml (1)
16-19: Issue-only guard and bot invocation are correctly wired.The job filter prevents PR-comment processing in this workflow and keeps responsibilities separated.
Also applies to: 38-45
.github/actions/bot-autoassign/base.py (1)
13-27: Initialization fallback is safe and practical.Gracefully setting
self.github/self.repotoNoneon missing env or init failure avoids hard crashes in action runs..github/workflows/bot-autoassign-pr-issue-link.yml (1)
4-6: Bot correctly handlespull_request_targetwith explicitclosedaction branching.The handler explicitly checks
self.event_name == "pull_request_target"(line 352) and routes tohandle_pull_request(), which branches on the action:opened/reopenedtrigger auto-assignment, andclosedtriggers unassignment viaunassign_issues_from_pr(). The workflow will not silently no-op..github/workflows/bot-autoassign-pr-reopen.yml (1)
50-53: Author gating correctly enforced inPRActivityBot.handle_contributor_activity().The workflow correctly triggers on any PR comment, but the author validation is properly in place before stale-label removal and issue reassignment. Line 130 of
pr_reopen_bot.pycompares the commenter against the PR author (if commenter != pr.user.login:) and returns early if they do not match, preventing any state changes by non-author users..github/actions/bot-autoassign/tests/test_pr_reopen_bot.py (1)
117-181: Good guard-path coverage for contributor activity flows.The tests around author checks, stale-label gating, non-PR comments, and unsupported events are solid and reduce regression risk in event routing.
Also applies to: 200-203
.github/actions/bot-autoassign/pr_reopen_bot.py (1)
82-85: Action-result handling in reopen flow looks correct now.Combining reassignment outcome with stale-label removal avoids false negatives when only label cleanup occurs.
.github/actions/bot-autoassign/issue_assignment_bot.py (1)
199-209:⚠️ Potential issue | 🔴 CriticalCross-repo issue references can map to the wrong local issue.
Line 199/Line 258 consume numeric issue IDs and Line 213/Line 264 fetch through
self.repo.get_issue(...). If PR text containsowner/repo#55and parsing drops repo context, the bot can act on local#55. Also, Line 266-268 cross-repo guard is ineffective after a local-repo lookup path.🛡️ Safer local-only extraction path (minimal hardening)
class IssueAssignmentBot(GitHubBot): + def _extract_local_linked_issues(self, pr_body): + # Intentionally accept only same-repo references like `Fixes `#123`` + pattern = ( + r"(?i)\b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?|related to)\s+#(\d+)\b" + ) + return sorted({int(m.group(1)) for m in re.finditer(pattern, pr_body or "")}) + def auto_assign_issues_from_pr(self, pr_number, pr_author, pr_body, max_issues=10): @@ - linked_issues = extract_linked_issues(pr_body) + linked_issues = self._extract_local_linked_issues(pr_body) @@ def unassign_issues_from_pr(self, pr_body, pr_author): @@ - linked_issues = extract_linked_issues(pr_body) + linked_issues = self._extract_local_linked_issues(pr_body) @@ - if ( - hasattr(issue, "repository") - and issue.repository.full_name != self.repository_name - ): - print( - f"Issue #{issue_number} is from a" - " different repository, skipping" - ) - continueAlso applies to: 258-268
⛔ Skipped due to learnings
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.
| except Exception as e: | ||
| print(f"Error running {bot_type} bot: {e}") | ||
| return 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Preserve traceback in the top-level exception handler.
The current handler hides stack context, which slows down debugging in workflow logs.
♻️ Proposed change
import sys
+import traceback
@@
except Exception as e:
print(f"Error running {bot_type} bot: {e}")
+ traceback.print_exc()
return 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-autoassign/__main__.py around lines 33 - 35, The
top-level exception handler catching Exception as e currently prints only e and
loses the traceback; update the handler around the block referencing bot_type to
preserve full stack information by either calling traceback.print_exc() or using
logging.exception()/logger.exception() to log a descriptive message including
bot_type and the exception (e) so the traceback is emitted to workflow
logs—modify the except Exception as e: handler to emit the traceback instead of
just print and then return 1.
| assignment_phrases = [ | ||
| "assign this issue to me", | ||
| "assign me", | ||
| "can i work on this", | ||
| "i would like to work on this", | ||
| "i want to work on this", | ||
| "please assign this to me", | ||
| "can you assign this to me", | ||
| ] | ||
| return any(phrase in comment_lower for phrase in assignment_phrases) |
There was a problem hiding this comment.
Assignment intent detection is too permissive and can false-trigger.
Line 22 uses raw substring matching, so comments like “please unassign me” still match “assign me”. This can trigger the wrong bot response.
🔧 Tighten intent matching
- return any(phrase in comment_lower for phrase in assignment_phrases)
+ if "unassign" in comment_lower:
+ return False
+ patterns = [
+ r"\bassign this issue to me\b",
+ r"\bassign me\b",
+ r"\bcan i work on this\b",
+ r"\bi would like to work on this\b",
+ r"\bi want to work on this\b",
+ r"\bplease assign this to me\b",
+ r"\bcan you assign this to me\b",
+ ]
+ return any(re.search(pattern, comment_lower) for pattern in patterns)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-autoassign/issue_assignment_bot.py around lines 13 - 22,
The current intent check uses raw substring matching against assignment_phrases
(variable assignment_phrases and the return any(...) line) which causes false
positives like "unassign me"; change the check to use stricter token/regex
matching—e.g., compile each phrase into a regex with word boundaries (\b) or
split the comment into tokens and match whole-word phrases, and also add
negative checks for common counterphrases like "unassign", "do not assign",
"don't assign" before returning true; update the any(...) logic to use these
word-boundary/negative filters so only true assignment intents trigger the bot.
| def process_stale_prs(self): | ||
| if not self.repo: | ||
| print("GitHub repository not initialized") | ||
| return | ||
| try: | ||
| open_prs = self.repo.get_pulls(state="open") | ||
| print(f"Found {open_prs.totalCount}" " open pull requests") | ||
| processed_count = 0 | ||
| for pr in open_prs: | ||
| try: | ||
| all_reviews = list(pr.get_reviews()) | ||
| last_changes_requested = self.get_last_changes_requested( | ||
| pr, all_reviews | ||
| ) | ||
| if not last_changes_requested: | ||
| continue | ||
| issue_comments = list(pr.get_issue_comments()) | ||
| review_comments = list(pr.get_review_comments()) | ||
| days_inactive = self.get_days_since_activity( | ||
| pr, | ||
| last_changes_requested, | ||
| issue_comments, | ||
| all_reviews, | ||
| review_comments, | ||
| ) | ||
| print( | ||
| f"PR #{pr.number}: {days_inactive}" | ||
| " days since contributor activity" | ||
| ) | ||
| if days_inactive >= self.DAYS_BEFORE_CLOSE: | ||
| if self.close_stale_pr(pr, days_inactive): | ||
| processed_count += 1 | ||
| elif days_inactive >= self.DAYS_BEFORE_UNASSIGN: | ||
| if not self.has_bot_comment( | ||
| pr, | ||
| "stale", | ||
| after_date=last_changes_requested, | ||
| issue_comments=issue_comments, | ||
| ): | ||
| if self.mark_pr_stale(pr, days_inactive): | ||
| processed_count += 1 | ||
| elif days_inactive >= self.DAYS_BEFORE_STALE_WARNING: | ||
| if not self.has_bot_comment( | ||
| pr, | ||
| "stale_warning", | ||
| after_date=last_changes_requested, | ||
| issue_comments=issue_comments, | ||
| ): | ||
| if self.send_stale_warning(pr, days_inactive): | ||
| processed_count += 1 | ||
| time.sleep(0.1) | ||
| except Exception as e: | ||
| print(f"Error processing" f" PR #{pr.number}: {e}") | ||
| continue | ||
| print(f"Processed {processed_count} stale PRs") | ||
| except Exception as e: | ||
| print(f"Error in process_stale_prs: {e}") | ||
|
|
There was a problem hiding this comment.
run() can report success even when stale processing fails fatally.
Line 382 catches fatal exceptions in process_stale_prs and only logs; Line 392 then returns True unconditionally from run(). This masks operational failures.
✅ Return execution status from `process_stale_prs` and propagate it
def process_stale_prs(self):
if not self.repo:
print("GitHub repository not initialized")
- return
+ return False
try:
@@
print(f"Processed {processed_count} stale PRs")
+ return True
except Exception as e:
print(f"Error in process_stale_prs: {e}")
+ return False
def run(self):
@@
print("Stale PR Management Bot starting...")
try:
- self.process_stale_prs()
- return True
+ return self.process_stale_prs()Also applies to: 385-393
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-autoassign/stale_pr_bot.py around lines 327 - 384,
process_stale_prs currently swallows fatal exceptions and run() returns True
regardless; change process_stale_prs to return a boolean status (True on
success, False on fatal error) and ensure all exception paths return False
(e.g., the outer try/except should log and return False), then update run() to
call process_stale_prs and propagate its result (return False if
process_stale_prs returned False) so the action reflects failures instead of
always returning True; locate and modify the process_stale_prs and run functions
to implement this control flow change.
| ("Fixes owner/repo#55", [55]), # cross-repo | ||
| ("Fixed #999", [999]), |
There was a problem hiding this comment.
Cross-repo parsing expectation here locks in unsafe behavior.
Line 89 currently expects owner/repo#55 to become local issue 55. That erases repository context and can drive incorrect local assignments in bot callers.
🧪 Suggested expectation change (local-only safety contract)
- ("Fixes owner/repo#55", [55]), # cross-repo
+ ("Fixes owner/repo#55", []), # cross-repo refs should not map to local issues📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ("Fixes owner/repo#55", [55]), # cross-repo | |
| ("Fixed #999", [999]), | |
| ("Fixes owner/repo#55", []), # cross-repo refs should not map to local issues | |
| ("Fixed `#999`", [999]), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py around
lines 89 - 90, The test currently treats the string "Fixes owner/repo#55" as
producing a local issue ID 55 which removes repo context; update the expectation
for that test tuple to reflect local-only parsing (i.e., no local IDs returned)
so the test enforces safe behavior—change the tuple ("Fixes owner/repo#55",
[55]) to ("Fixes owner/repo#55", []) in the test data used by
test_issue_assignment_bot.py (the list of (message, expected_ids) cases).
| **Secrets** | ||
|
|
||
| - ``OPENWISP_BOT_APP_ID`` (required): OpenWISP Bot GitHub App ID. | ||
| - ``OPENWISP_BOT_PRIVATE_KEY`` (required): OpenWISP Bot GitHub App private | ||
| key. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify “required secrets” vs bot runtime environment.
This section currently implies the bot directly requires OPENWISP_BOT_APP_ID / OPENWISP_BOT_PRIVATE_KEY. In practice, those are used to mint GITHUB_TOKEN; the bot runtime consumes GITHUB_TOKEN, REPOSITORY, and GITHUB_EVENT_NAME. A short note here would prevent setup confusion.
Also applies to: 122-126
🤖 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 69 - 73, The Secrets
section incorrectly implies the bot runtime consumes OPENWISP_BOT_APP_ID and
OPENWISP_BOT_PRIVATE_KEY directly; update the text to state that
OPENWISP_BOT_APP_ID and OPENWISP_BOT_PRIVATE_KEY are required only to mint a
GITHUB_TOKEN, and that the bot runtime itself needs GITHUB_TOKEN plus REPOSITORY
and GITHUB_EVENT_NAME environment variables; adjust the prose around the listed
symbols (OPENWISP_BOT_APP_ID, OPENWISP_BOT_PRIVATE_KEY) and add a short note
mentioning the runtime requirements (GITHUB_TOKEN, REPOSITORY,
GITHUB_EVENT_NAME) so readers won’t confuse setup vs runtime secrets.
| To enable the auto-assignment bot in any OpenWISP repository, copy the | ||
| workflow files from ``.github/workflows/`` (``bot-autoassign-issue.yml``, | ||
| ``bot-autoassign-pr-issue-link.yml``, ``bot-autoassign-pr-reopen.yml``, | ||
| ``bot-autoassign-stale-pr.yml``): |
There was a problem hiding this comment.
Setup instructions are incomplete and will break execution.
Line 77–80 says to copy only workflow files, but Line 127–128 executes .github/actions/bot-autoassign/__main__.py. Without also copying .github/actions/bot-autoassign/, the workflow fails at runtime (file not found). Please update this section to require copying both workflows and bot scripts (or document a reusable-workflow-only path).
Also applies to: 127-128
🤖 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 77 - 80, The docs
currently instruct copying only the workflow files from .github/workflows (e.g.,
bot-autoassign-issue.yml, bot-autoassign-pr-issue-link.yml,
bot-autoassign-pr-reopen.yml, bot-autoassign-stale-pr.yml) but later the
workflow executes .github/actions/bot-autoassign/__main__.py; update the
documentation to require copying the .github/actions/bot-autoassign/ directory
(all bot scripts) alongside the workflows or alternatively document a separate
“reusable-workflow-only” setup that removes references to
.github/actions/bot-autoassign/__main__.py so the workflow won’t fail at
runtime.
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