[chore] Added reusable active autoassign bots workflow and fixed exit codes#613
[chore] Added reusable active autoassign bots workflow and fixed exit codes#613Eeshu-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:
📝 WalkthroughWalkthroughHandlers in .github/actions/bot-autoassign (issue_assignment_bot.py, pr_reopen_bot.py, stale_pr_bot.py) were updated to return True in many early-return/no-op/error branches where they previously returned False; corresponding unit tests were adjusted to expect True. A reusable GitHub Actions workflow was added at .github/workflows/reusable-bot-autoassign.yml to generate a GitHub App token, checkout the repository, set up Python, install dependencies, and run the bot CLI with BOT_COMMAND and optional GITHUB_EVENT_PATH. Documentation was updated to reference and demonstrate using the reusable workflow. Sequence Diagram(s)mermaid Workflow->>TokenAction: request app token (APP_ID, PRIVATE_KEY) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The changes in this PR correctly fix a semantic issue where bot scripts were returning failure exit codes when they simply had nothing to do (e.g., comment on a PR instead of an issue, unsupported event type). Now The new reusable workflow ( Files Reviewed (5 files)
|
coderabbitai
bot
added
enhancement
github_actions
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/workflows/reusable-bot-autoassign.yml:
- Around line 48-51: The workflow currently passes the bot_command input
unquoted to the Python script; update both invocations in the
reusable-bot-autoassign.yml snippet to quote ${{ inputs.bot_command }} (e.g.,
use "${{ inputs.bot_command }}") so that the bot_command argument is passed
robustly even if it contains whitespace or special characters, keeping the
existing conditional that appends "$GITHUB_EVENT_PATH" intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e565351-663d-4043-b38a-a25e2515a1b3
📒 Files selected for processing (5)
.github/actions/bot-autoassign/issue_assignment_bot.py.github/actions/bot-autoassign/pr_reopen_bot.py.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py.github/workflows/reusable-bot-autoassign.yml
📜 Review details
⏰ 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). (7)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 0
File: :0-0
Timestamp: 2026-03-05T20:55:29.968Z
Learning: For the openwisp/openwisp-utils repository (and the broader openwisp org), the maintainers have explicitly decided to use mutable version tags (e.g. `v2`, `v6`) for GitHub Actions and to use `ref: master` for internal script checkouts. Do NOT flag unpinned action versions or `master`/version-tag usage as security issues in any openwisp-utils workflow file. The maintainers are aware of the trade-offs and are intentionally fine with this approach.
📚 Learning: 2026-03-05T14:23:55.528Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Applied to files:
.github/workflows/reusable-bot-autoassign.yml.github/actions/bot-autoassign/pr_reopen_bot.py
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-autoassign/pr_reopen_bot.py
🔇 Additional comments (5)
.github/actions/bot-autoassign/pr_reopen_bot.py (1)
92-92: LGTM! Return value semantics correctly updated for skip paths.The change from
return Falsetoreturn Truefor non-critical skip scenarios (unsupported events, comments on issues vs PRs, non-author comments, non-stale PRs) aligns with the PR objective. These paths represent successful "no-op" handling rather than errors, so returningTrue(exit code 0) prevents false-positive CI failures.The error paths (missing payload, exceptions, missing required data) still correctly return
False.Also applies to: 116-116, 120-120, 124-124, 172-172
.github/actions/bot-autoassign/issue_assignment_bot.py (1)
276-276: LGTM! Consistent return value semantics for non-error skip paths.The updated return values correctly distinguish between:
- True: Event was processed or intentionally skipped (comment on PR, no assignment request, unsupported action/event)
- False: Actual errors (missing payload, exceptions, missing required data)
This prevents CI failures when the bot encounters events it doesn't need to act on, which is the intended behavior per the PR objectives.
Also applies to: 288-288, 314-314, 331-331
.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py (1)
108-112: LGTM! Test expectations correctly updated to match implementation.The test assertions properly reflect the new return value semantics:
- Unsupported events and non-critical skip paths now expect
True- Error conditions (e.g.,
test_handle_contributor_activity_no_payloadat line 197) still correctly expectFalseThis ensures the tests validate the intended behavior where skipping irrelevant events is a success, not a failure.
Also applies to: 147-163, 165-180, 182-193, 199-202
.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py (1)
278-294: LGTM! Test expectations correctly aligned with implementation changes.The test updates properly validate:
- Skip paths (
test_skip_pr_comment,test_non_assignment_comment,test_unsupported_action,test_unsupported_event) now expectTrue- Error paths (
test_no_payload,test_no_github_client) still correctly expectFalseThis maintains proper test coverage for both success and failure scenarios.
Also applies to: 296-307, 376-388, 431-434
.github/workflows/reusable-bot-autoassign.yml (1)
1-52: LGTM! Well-structured reusable workflow.The workflow correctly:
- Generates a GitHub App token for authentication
- Checks out openwisp-utils and sets up Python 3.13
- Installs dependencies via the
[github_actions]extras- Conditionally passes
GITHUB_EVENT_PATHwhen available- Provides the required environment variables (
GITHUB_TOKEN,REPOSITORY,GITHUB_EVENT_NAME) that match whatGitHubBotbase class expectsThe invocation pattern matches the existing
bot-autoassign-issue.ymlworkflow, ensuring compatibility. Based on learnings, the use of mutable version tags (v2,v6) andref: masteris intentional and acceptable for this repository.
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable GitHub Actions workflow (reusable-bot-autoassign.yml) that other OpenWISP repositories can call via workflow_call to run the auto-assignment bots, avoiding duplication of the full bot workflow boilerplate across repos. It also fixes the Python bot scripts to return True (exit 0) instead of False (exit 1) when skipping irrelevant or inapplicable events, preventing false-positive CI failures.
Changes:
- Added a new reusable workflow (
reusable-bot-autoassign.yml) that checks outopenwisp-utilsand runs any bot command viaworkflow_call. - Fixed return values in
issue_assignment_bot.pyandpr_reopen_bot.pyso that "skip" conditions (unsupported events, non-applicable payloads) returnTrueinstead ofFalse. - Updated test assertions in
test_issue_assignment_bot.pyandtest_pr_reopen_bot.pyto reflect the corrected return semantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/reusable-bot-autoassign.yml |
New reusable workflow for running bot scripts in other repos |
.github/actions/bot-autoassign/issue_assignment_bot.py |
Fixed skip-case return values from False to True |
.github/actions/bot-autoassign/pr_reopen_bot.py |
Fixed skip-case return values from False to True |
.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py |
Updated test assertions to match corrected return semantics |
.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py |
Updated test assertions to match corrected return semantics |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Eeshu-Yadav
changed the title
Eeshu-Yadav
force-pushed
the
feature/reusable-autoassign-workflow
branch
from
686b04a to
d9ec4a7
Compare
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
@Eeshu-Yadav can you please update the bot testin ground accordingly so we can test this there before merging?
What about the worfklows added before? And what about the docs? Does the docs need to be updated?
|
Eeshu-Yadav
force-pushed
the
feature/reusable-autoassign-workflow
branch
from
836205d to
a483dbe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/developer/reusable-github-utils.rst`:
- Around line 89-98: The reusable workflow incorrectly maps inputs.bot_command
to BOT_TOKEN and never sets BOT_COMMAND, so downstream examples passing
bot_command fail; update reusable-bot-autoassign.yml to set the environment
variables correctly by assigning BOT_COMMAND: ${{ inputs.bot_command }} (and
ensure BOT_TOKEN is assigned from the appropriate input, e.g., BOT_TOKEN: ${{
inputs.bot_token }}), and keep invoking the script with __main__.py
"$BOT_COMMAND" (or pass the input as an argument) so the script receives the
command under the name it expects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5ae685e-7d49-4cd6-9d22-610630ee3ac2
📒 Files selected for processing (1)
docs/developer/reusable-github-utils.rst
📜 Review details
⏰ 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.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.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.0.0
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T14:23:55.528Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Applied to files:
docs/developer/reusable-github-utils.rst
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
docs/developer/reusable-github-utils.rst
🔇 Additional comments (1)
docs/developer/reusable-github-utils.rst (1)
118-123: This follows the project maintainers' explicit approach for openwisp-utils.While GitHub best practices recommend pinning to immutable refs (commit SHAs) for production stability, the openwisp-utils maintainers have explicitly decided to use
@masterand mutable version tags for their workflows, and have documented awareness of the trade-offs. This approach is intentional for the project.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-autoassign/issue_assignment_bot.py:
- Around line 289-291: The three broad except Exception as e blocks that
currently just print messages like "Error handling issue comment: {e}" and then
return True should be changed to not mask real failures: for unexpected
exceptions either re-raise the exception or return False so the workflow fails
(and include the full traceback via logging.exception or similar), while keeping
intentional skips returning True where the code explicitly detects
non-assignment/unsupported events; update the except blocks (the ones that print
"Error handling issue comment: {e}" and the two other similar exception
handlers) to differentiate expected skips vs unexpected errors, log full error
details, and adjust the return value accordingly (or add a clear comment
documenting why an exception would be swallowed if you choose to preserve the
current behavior).
In @.github/actions/bot-autoassign/pr_reopen_bot.py:
- Around line 101-104: The method handle_contributor_activity currently returns
True when self.event_payload is missing, which is inconsistent with
handle_pr_reopen (which returns False); change handle_contributor_activity to
return False in the empty-payload branch so both methods treat a missing
event_payload the same way (i.e., treat it as an unexpected error case), keeping
the check on self.event_payload and the print("No event payload available")
intact.
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 321-324: The check in process_stale_prs currently prints "GitHub
repository not initialized" and returns True, which is inconsistent with run()'s
behavior for initialization failures; change process_stale_prs (method name) to
return False when self.repo is not set so it signals failure like run() does,
keeping the existing print/log message unchanged.
In @.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py:
- Around line 195-197: The test shows handle_contributor_activity() returns True
when called with no payload but handle_pr_reopen() returns False for missing
payload; either add a brief inline comment in the PRActivityBot class explaining
why handle_contributor_activity is intentionally permissive, referencing the
method name handle_contributor_activity to justify this behavior, or change the
implementation of handle_contributor_activity to mirror handle_pr_reopen's
behavior (return False on missing payload) and update the test accordingly;
ensure you modify the method body (and its test) or add the explanatory comment
near the handle_contributor_activity definition so future readers understand the
discrepancy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: df45aca3-0767-46d2-9397-0c3b7c9fb48c
📒 Files selected for processing (5)
.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_pr_reopen_bot.py.github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📜 Review details
⏰ 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.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | 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.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/actions/bot-autoassign/stale_pr_bot.py
🔇 Additional comments (5)
.github/actions/bot-autoassign/issue_assignment_bot.py (1)
329-334: LGTM for unsupported event handling.Returning
Truefor unsupported event types correctly signals that the bot handled the situation (by intentionally skipping) rather than failing. This prevents false-positive CI failures when the workflow is triggered by events the bot doesn't handle..github/actions/bot-autoassign/stale_pr_bot.py (1)
144-147: LGTM for already-closed PR handling.Returning
Truewhen the PR is already closed is correct - this is an expected condition, not an error..github/actions/bot-autoassign/pr_reopen_bot.py (1)
114-124: LGTM for early-exit skip conditions.Returning
Truefor these conditions is appropriate:
- Comment is on an issue, not a PR (line 116)
- Comment not from PR author (line 120)
- PR is not stale (line 124)
These are "not my job" conditions where the bot correctly identifies the event doesn't require action.
.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py (1)
108-111: LGTM for unsupported event test updates.Both
test_run_unsupported_eventtests correctly expectTrue, matching the updated behavior where unsupported events are treated as handled (skip) rather than errors.Also applies to: 199-202
.github/actions/bot-autoassign/tests/test_stale_pr_bot.py (1)
230-236: LGTM for already-closed PR test update.The test correctly expects
Truewhen attempting to close an already-closed PR, matching the updated implementation. The assertions thatcreate_issue_commentandeditare not called remain correct - the bot properly skips redundant actions.
Eeshu-Yadav
force-pushed
the
feature/reusable-autoassign-workflow
branch
from
c5b0652 to
984dcd6
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Selenium Test FailureHello @Eeshu-Yadav, There is one test failure in the CI logs:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-autoassign/issue_assignment_bot.py:
- Around line 282-284: The current checks that print "Missing required comment
data" and return True silently treat malformed event payloads as success; change
these to treat missing required fields as failures by returning False (or
raising an error) instead of True. Specifically update the branch that checks
comment_body, commenter, issue_number to return False when any are missing, and
similarly update the branch checking pr_number and pr_author (the other block
noted) to return False; locate these checks by the variable names comment_body,
commenter, issue_number, pr_number, and pr_author and adjust the return value so
malformed payloads do not exit 0.
In @.github/actions/bot-autoassign/pr_reopen_bot.py:
- Around line 70-72: The branch that checks required payload fields uses `if not
all([pr_number, pr_author]):` but returns True (treating it as a skip); change
it to return False so missing PR/comment fields are treated as failures. Update
the `process`/handler function where `pr_number` and `pr_author` are validated
to log the missing-data case and return False instead of True, and make the same
change for the analogous check around the other occurrence (the block referenced
at lines 111-113) to ensure both validation failures return False rather than
True.
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 149-151: The current checks that set pr_author = pr.user.login if
pr.user else None and then return True when missing cause PRs with no author to
be treated as successfully processed; change those branches in
send_stale_warning, mark_pr_stale, and close_stale_pr so they do not return True
for missing authors — either (A) return False so process_stale_prs knows the PR
was not handled, or preferably (B) continue processing using a generic
salutation (e.g., set pr_author to "there" or "maintainer") and proceed with the
warning/unassign/close logic; update the pr_author assignment sites (where
pr.user is checked) to implement one of these fixes consistently and ensure
process_stale_prs receives a proper success/failure result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e0110ea-60fd-40d1-83e3-6e80480fbac9
📒 Files selected for processing (8)
.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/workflows/reusable-bot-autoassign.ymldocs/developer/reusable-github-utils.rst
📜 Review details
⏰ 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). (2)
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-autoassign/tests/test_pr_reopen_bot.pydocs/developer/reusable-github-utils.rst.github/workflows/reusable-bot-autoassign.yml
📚 Learning: 2026-03-05T14:23:55.528Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Applied to files:
docs/developer/reusable-github-utils.rst.github/workflows/reusable-bot-autoassign.yml
🔇 Additional comments (1)
.github/workflows/reusable-bot-autoassign.yml (1)
27-32: The current approach is intentional per project guidelines.The maintainers of openwisp-utils have explicitly decided to use
ref: masterfor internal script checkouts in workflow files. This is an intentional design choice, and the trade-offs are understood and accepted.> Likely an incorrect or invalid review comment.
| if not all([comment_body, commenter, issue_number]): | ||
| print("Missing required comment data") | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Keep malformed event payloads as failures.
These branches now treat missing required webhook fields as a successful skip. That makes sense for irrelevant events, but not for malformed issue_comment / pull_request_target payloads. If issue_number, commenter, pr_number, or pr_author is missing, something is broken and the workflow should not exit 0 silently.
🛠️ Proposed fix
if not all([comment_body, commenter, issue_number]):
print("Missing required comment data")
- return True
+ return False
@@
if not all([pr_number, pr_author]):
print("Missing required PR data")
- return True
+ return FalseAlso applies to: 303-305
🤖 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 282 -
284, The current checks that print "Missing required comment data" and return
True silently treat malformed event payloads as success; change these to treat
missing required fields as failures by returning False (or raising an error)
instead of True. Specifically update the branch that checks comment_body,
commenter, issue_number to return False when any are missing, and similarly
update the branch checking pr_number and pr_author (the other block noted) to
return False; locate these checks by the variable names comment_body, commenter,
issue_number, pr_number, and pr_author and adjust the return value so malformed
payloads do not exit 0.
| if not all([pr_number, pr_author]): | ||
| print("Missing required PR data") | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Treat missing payload fields as failures, not handled skips.
These branches now return True when required PR/comment fields are absent. That is different from an intentional skip and will mask broken payload loading or event-shape regressions. The permissive True paths for "not a PR", "not author", and "not stale" are fine; these data-validation failures should still be False.
🛠️ Proposed fix
if not all([pr_number, pr_author]):
print("Missing required PR data")
- return True
+ return False
@@
if not all([pr_number, commenter]):
print("Missing required comment data")
- return True
+ return FalseAlso applies to: 111-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-autoassign/pr_reopen_bot.py around lines 70 - 72, The
branch that checks required payload fields uses `if not all([pr_number,
pr_author]):` but returns True (treating it as a skip); change it to return
False so missing PR/comment fields are treated as failures. Update the
`process`/handler function where `pr_number` and `pr_author` are validated to
log the missing-data case and return False instead of True, and make the same
change for the analogous check around the other occurrence (the block referenced
at lines 111-113) to ensure both validation failures return False rather than
True.
| pr_author = pr.user.login if pr.user else None | ||
| if not pr_author: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Don't report missing-author PRs as successfully processed.
These branches skip the actual warning/stale/close work but still return True, so process_stale_prs() counts the PR as processed. A PR whose author account is unavailable will therefore never be warned, unassigned, or closed, and the logs will still say it was handled.
At minimum, this should return False; better yet, keep processing with a generic salutation instead of requiring pr.user.login.
🛠️ Minimal safe fix
pr_author = pr.user.login if pr.user else None
if not pr_author:
- return True
+ return FalseApply the same change in close_stale_pr, mark_pr_stale, and send_stale_warning.
Also applies to: 211-213, 274-276
🤖 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 149 - 151, The
current checks that set pr_author = pr.user.login if pr.user else None and then
return True when missing cause PRs with no author to be treated as successfully
processed; change those branches in send_stale_warning, mark_pr_stale, and
close_stale_pr so they do not return True for missing authors — either (A)
return False so process_stale_prs knows the PR was not handled, or preferably
(B) continue processing using a generic salutation (e.g., set pr_author to
"there" or "maintainer") and proceed with the warning/unassign/close logic;
update the pr_author assignment sites (where pr.user is checked) to implement
one of these fixes consistently and ensure process_stale_prs receives a proper
success/failure result.
This PR adds a reusable GitHub Actions workflow for the auto-assignment bots to reduce boilerplate code across OpenWISP repositories. It also includes fixes for the Python script return values so they exit successfully when skipping irrelevant events, avoiding false-positive CI failures.