Skip to content

[chores:fix] Auto-assign bot: wait for maintainers before complaining#625

Merged
nemesifier merged 2 commits intomasterfrom
auto-assign-bot-too-eager
Mar 16, 2026
Merged

[chores:fix] Auto-assign bot: wait for maintainers before complaining#625
nemesifier merged 2 commits intomasterfrom
auto-assign-bot-too-eager

Conversation

@nemesifier
Copy link
Member

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • N/A I have updated the documentation.

Description of Changes

This is very wrong:
#604 (comment)

The bot complained with a contributor that the PR has had no activity, but no maintainer has replied since the contributor has updated his PR.

The bot shouldn't do this, it should only complain when maintainers have replied after the latest changes and there hasn't been any follow up.

I've seen a PR being closed as well recently due to the auto-assign bot, which didn't have to be closed because there were no updates from maintainers, the PR stalled but not because of the contributor, when this happens the bot should not close anything.

This patch fixes this.

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 24790e3e-6431-4b50-a5d3-87377c846c32

📥 Commits

Reviewing files that changed from the base of the PR and between d547ce8 and 7b52ea1.

📒 Files selected for processing (2)
  • .github/actions/bot-autoassign/stale_pr_bot.py
  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📜 Recent 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.10 | django~=4.2.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.2.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.0.0
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=5.0.0
  • 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.1.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=4.2.0
🧰 Additional context used
🧠 Learnings (6)
📓 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.
📚 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/actions/bot-autoassign/stale_pr_bot.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
🔇 Additional comments (8)
.github/actions/bot-autoassign/stale_pr_bot.py (5)

7-8: LGTM! Well-defined maintainer roles constant.

Using a frozenset for MAINTAINER_ROLES is a good choice for immutability and O(1) membership checks. The inclusion of COLLABORATOR alongside OWNER and MEMBER aligns with the project's established conventions. Based on learnings: the maintainer has explicitly decided that COLLABORATOR should be allowed for non-destructive bot workflows.


18-64: LGTM! Full iteration addresses previous truncation concerns.

The method now iterates through all commits, comments, and reviews without artificial limits (no deque(maxlen=...) or [-N:] slicing). This ensures contributor activity won't be missed regardless of timeline volume.


66-89: LGTM! Clean refactor using new helper.

The fallback logic is correct: if the author has activity after changes were requested, measure from that date; otherwise measure from the last_changes_requested date. Returning 0 on error is a safe default that prevents erroneous stale actions.


91-157: LGTM! Past review concerns properly addressed.

The implementation now:

  1. Consistently applies bot filtering (user.type != "Bot") across all three activity sources (issue comments, review comments, and reviews)
  2. Strictly requires author_association in MAINTAINER_ROLES to clear the waiting state—random community comments no longer count

This correctly implements the "waiting for maintainer" semantics described in the PR objectives.


418-429: LGTM! Clean integration of maintainer-wait check.

The check is well-placed after fetching all required data (reusing the cached lists) and before any stale actions. This efficiently prevents warnings, stale marking, or closure when the PR is awaiting maintainer input.

.github/actions/bot-autoassign/tests/test_stale_pr_bot.py (3)

107-117: LGTM! Clean test helper method.

The _make_pr helper provides a well-structured mock PR object with sensible defaults for all activity sources. This reduces boilerplate and makes individual tests more focused.


118-228: LGTM! Comprehensive test coverage addressing past review concerns.

The test suite covers:

  • Core waiting/not-waiting scenarios
  • Bot comments being ignored (line 172-188)
  • Non-maintainer comments being ignored (line 190-205) — addresses past review request
  • High-volume timelines with 60 subsequent commits (line 207-227) — addresses past review request

All tests use fixed datetime values, making them deterministic as recommended in the learnings.


362-390: LGTM! Good integration test for skip behavior.

The test validates the end-to-end flow: a PR with contributor activity but no maintainer response is correctly skipped without warnings, stale marking, or closure. The datetime patching ensures deterministic behavior.


📝 Walkthrough

Walkthrough

This pull request updates the stale PR bot to detect when a pull request is awaiting maintainer action and to skip stale-processing in that case. It adds MAINTAINER_ROLES, _get_last_author_activity() to compute the author's latest activity after a given date, and is_waiting_for_maintainer() to determine whether maintainers have acted since the author’s last activity. process_stale_prs() is updated to bypass processing for PRs that are waiting for maintainer input. Tests covering maintainer-wait scenarios and ensuring skipped processing were added.

Sequence Diagram(s)

sequenceDiagram
    participant Author as Author
    participant Maintainer as Maintainer
    participant GitHub as GitHub API
    participant Bot as StalePRBot

    Author->>GitHub: push commit / post comment after CHANGES_REQUESTED
    Bot->>GitHub: fetch commits, issue comments, review comments, reviews
    Bot->>Bot: _get_last_author_activity(pr, after_date)
    Bot->>Bot: is_waiting_for_maintainer(pr, last_changes_requested)
    alt Maintainer acted after author's last activity
        Bot->>Bot: continue stale processing (warn/stale/close)
    else No maintainer action
        Bot->>Bot: skip processing for this PR
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title includes a type prefix [chores:fix] and clearly describes the main change: preventing the bot from flagging PRs as stale when awaiting maintainer response.
Description check ✅ Passed The description includes completed checklist items, explains the issue and fix clearly with references, but lacks a specific issue reference and screenshot section.
Bug Fixes ✅ Passed The pull request correctly fixes the root cause by introducing an is_waiting_for_maintainer() method that checks if a maintainer has acted since the contributor's last update, integrated into process_stale_prs() to skip stale actions when true. A comprehensive regression test suite with 8 test cases covers the exact bug scenario and edge cases.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch auto-assign-bot-too-eager
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added github_actions Pull requests that update GitHub Actions code helper-bots Helper bots, release management automation labels Mar 16, 2026
@coveralls
Copy link

coveralls commented Mar 16, 2026

Coverage Status

coverage: 97.348%. remained the same
when pulling 7b52ea1 on auto-assign-bot-too-eager
into fe1cc60 on master.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/stale_pr_bot.py:
- Around line 32-33: The activity detection currently truncates
commits/comments/reviews (e.g., using deque(..., maxlen=50) or slicing last N)
in _get_last_author_activity which can omit valid older responses; remove the
lossy truncation and iterate the full pr.get_commits(), pr.get_issue_comments(),
and pr.get_reviews() iterables instead (replace deque(..., maxlen=...) and any
[-N:] slices with full iteration or list() of the generator), filter by author
and timestamp as before, and compute last_author_activity from the complete sets
so no contributor activity after last_changes_requested is missed.
- Around line 121-157: The function currently treats any non-author (and only
some bot-filtered) activity as maintainer response; update the checks so only
actual maintainer/collaborator/owner activity clears the waiting state and apply
bot filtering consistently across issue comments, review comments, and reviews.
Concretely, when iterating issue_comments, review_comments (from
pr.get_review_comments()) and reviews (from pr.get_reviews()), require
comment.user or review.user to exist, ensure user.type != "Bot", and check
comment.user.author_association (or review.user.author_association) is one of
the maintainer roles like "MEMBER","COLLABORATOR","OWNER" (or whatever repo
association values your GitHub lib exposes) before comparing
created_at/submitted_at to last_author_activity; mirror that same logic for all
three loops and remove the current loose non-author checks so only maintainer
associations can return False.

In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py:
- Around line 107-185: Add two tests to TestIsWaitingForMaintainer: one named
like test_non_maintainer_comment_after_contributor that creates a contributor
commit (set pr.get_commits.return_value) and then an issue comment from a
non-maintainer human (comment.user.login not in maintainers,
comment.user.type="User") returned by pr.get_issue_comments, and assert
bot.is_waiting_for_maintainer(pr, last_cr) remains True; and another named like
test_contributor_activity_outside_timeline_window that simulates a contributor
commit older than the most-recent timeline window by populating the
timeline-related sources the implementation checks (e.g., pr.get_commits and
whatever timeline method your code uses such as
pr.get_issue_comments/pr.get_reviews) with many newer events so the contributor
commit is outside the 20/50 most-recent events, then assert the expected result
from is_waiting_for_maintainer; use the existing _make_pr helper and the
StalePRBot.is_waiting_for_maintainer entry point to locate where to wire the
mocks.
🪄 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: 247d7872-ba0e-40d1-adb8-ca9ac19d96af

📥 Commits

Reviewing files that changed from the base of the PR and between fe1cc60 and d547ce8.

📒 Files selected for processing (2)
  • .github/actions/bot-autoassign/stale_pr_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~=4.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.11 | 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~=5.2.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.11 | django~=5.0.0
  • GitHub Check: Python==3.10 | django~=5.0.0
  • GitHub Check: Python==3.11 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=5.0.0
🧰 Additional context used
🧠 Learnings (5)
📓 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: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
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features

Applied to files:

  • .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
🔇 Additional comments (2)
.github/actions/bot-autoassign/stale_pr_bot.py (1)

423-434: Good skip-path for maintainer-wait state.

Skipping stale processing when is_waiting_for_maintainer(...) is true cleanly matches the intended behavior.

.github/actions/bot-autoassign/tests/test_stale_pr_bot.py (1)

319-347: Nice deterministic coverage for the skip-while-waiting path.

This test correctly validates that no stale warning/mark/close actions are executed when the PR is waiting for maintainer response.

@github-project-automation github-project-automation bot moved this from To do (general) to In progress in OpenWISP Contributor's Board Mar 16, 2026
@github-project-automation github-project-automation bot moved this from In progress to Reviewer approved in OpenWISP Priorities for next releases Mar 16, 2026
@nemesifier nemesifier merged commit 29fd33b into master Mar 16, 2026
36 checks passed
@nemesifier nemesifier deleted the auto-assign-bot-too-eager branch March 16, 2026 20:02
@github-project-automation github-project-automation bot moved this from Reviewer approved to Done in OpenWISP Priorities for next releases Mar 16, 2026
@github-project-automation github-project-automation bot moved this from In progress to Done in OpenWISP Contributor's Board Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug github_actions Pull requests that update GitHub Actions code helper-bots Helper bots, release management automation

Development

Successfully merging this pull request may close these issues.

2 participants