[ci] Disabled auto-close PRs#673
Conversation
📝 WalkthroughWalkthroughThis pull request temporarily disables the automatic closing of stale pull requests. The Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ 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 Files Reviewed (2 files)
The PR is a clean temporary fix that disables the problematic auto-close functionality. The early return at the start of Reviewed by kimi-k2.5-0127 · 78,302 tokens |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 212-217: The unconditional early return that prints "Auto-close
currently disabled" should be replaced with a feature gate so the remaining
auto-close logic (below the current return) stays reachable; remove the
hardcoded "return False" and instead check a boolean flag (e.g., an env var like
ENABLE_AUTO_CLOSE or a config value) before returning—if the flag is false, log
the same message and return False, otherwise allow execution to continue into
the existing auto-close code that follows the block referencing pr.number.
Update any imports if needed (os or config) and ensure the new conditional uses
that flag to control flow rather than an unconditional return.
In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py:
- Line 332: Remove the `@pytest.mark.skip` on the TestCloseStalePR suite and
change the tests to assert the temporary disabled contract: call the
close_stale_pr (or the test helper that invokes it) and assert it returns False,
and assert the side-effect mocks (e.g., mock_post_comment, mock_edit_state,
mock_unassign or whatever fixtures are used in the test) were not called; keep
the test class name TestCloseStalePR and the close_stale_pr stub reference so
the assertions validate no comment was posted, no state was edited, and no
unassign occurred.
🪄 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: bcb0c182-8f10-45b3-879d-335dbab45987
📒 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). (16)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | 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.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~=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
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
| # TEMPORARY: auto-close disabled. The stale-detection heuristic | ||
| # has been closing PRs that are merely blocked by bot reviews | ||
| # (or by reviews the same reviewer later approved). The proper | ||
| # fix lives in PR #668; until it lands, no PR is auto-closed. | ||
| print(f"Auto-close currently disabled, skipping PR #{pr.number}") | ||
| return False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace unconditional early return with an explicit feature gate.
At Line 217, the unconditional return False makes Lines 218-280 permanently unreachable. That keeps dead logic in place and increases risk of silent drift while auto-close is disabled.
Proposed refactor
class StalePRBot(GitHubBot):
def __init__(self):
super().__init__()
self.DAYS_BEFORE_STALE_WARNING = 7
self.DAYS_BEFORE_UNASSIGN = 14
self.DAYS_BEFORE_CLOSE = 60
+ self.ENABLE_AUTO_CLOSE = False # temporary kill-switch
def close_stale_pr(self, pr, days_inactive):
- # TEMPORARY: auto-close disabled. The stale-detection heuristic
- # has been closing PRs that are merely blocked by bot reviews
- # (or by reviews the same reviewer later approved). The proper
- # fix lives in PR `#668`; until it lands, no PR is auto-closed.
- print(f"Auto-close currently disabled, skipping PR #{pr.number}")
- return False
+ if not self.ENABLE_AUTO_CLOSE:
+ print(f"Auto-close currently disabled, skipping PR #{pr.number}")
+ return False
if pr.state == "closed":
print(f"PR #{pr.number} is already closed, skipping")
return True📝 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.
| # TEMPORARY: auto-close disabled. The stale-detection heuristic | |
| # has been closing PRs that are merely blocked by bot reviews | |
| # (or by reviews the same reviewer later approved). The proper | |
| # fix lives in PR #668; until it lands, no PR is auto-closed. | |
| print(f"Auto-close currently disabled, skipping PR #{pr.number}") | |
| return False | |
| if not self.ENABLE_AUTO_CLOSE: | |
| print(f"Auto-close currently disabled, skipping PR #{pr.number}") | |
| return False |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/bot-autoassign/stale_pr_bot.py around lines 212 - 217, The
unconditional early return that prints "Auto-close currently disabled" should be
replaced with a feature gate so the remaining auto-close logic (below the
current return) stays reachable; remove the hardcoded "return False" and instead
check a boolean flag (e.g., an env var like ENABLE_AUTO_CLOSE or a config value)
before returning—if the flag is false, log the same message and return False,
otherwise allow execution to continue into the existing auto-close code that
follows the block referencing pr.number. Update any imports if needed (os or
config) and ensure the new conditional uses that flag to control flow rather
than an unconditional return.
| mock_issue.remove_from_assignees.assert_called_once_with("testuser") | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="Auto-close temporarily disabled; see close_stale_pr stub.") |
There was a problem hiding this comment.
Do not skip the whole close-PR suite; assert disabled behavior instead.
Skipping TestCloseStalePR at Line 332 removes coverage entirely. Keep tests running by asserting the temporary contract (auto-close disabled → returns False, no comment posted, no state edit, no unassign).
Suggested direction
-@pytest.mark.skip(reason="Auto-close temporarily disabled; see close_stale_pr stub.")
class TestCloseStalePR:
- def test_success(self, bot_env):
+ def test_disabled_no_side_effects(self, bot_env):
bot = StalePRBot()
mock_pr = Mock()
+ mock_pr.number = 1
mock_pr.body = "Fixes `#123`"
mock_pr.user.login = "testuser"
mock_pr.state = "open"
- ...
- assert bot.close_stale_pr(mock_pr, 60)
- mock_pr.create_issue_comment.assert_called_once()
- ...
- mock_pr.edit.assert_called_once_with(state="closed")
- ...
+ assert bot.close_stale_pr(mock_pr, 60) is False
+ mock_pr.create_issue_comment.assert_not_called()
+ mock_pr.edit.assert_not_called()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py at line 332,
Remove the `@pytest.mark.skip` on the TestCloseStalePR suite and change the tests
to assert the temporary disabled contract: call the close_stale_pr (or the test
helper that invokes it) and assert it returns False, and assert the side-effect
mocks (e.g., mock_post_comment, mock_edit_state, mock_unassign or whatever
fixtures are used in the test) were not called; keep the test class name
TestCloseStalePR and the close_stale_pr stub reference so the assertions
validate no comment was posted, no state was edited, and no unassign occurred.
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
Quickfix to disable auto-close PRs which is causing issues.