Skip to content

[ci] Disabled auto-close PRs #673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nemesifier merged 1 commit into from
May 9, 2026
Merged

[ci] Disabled auto-close PRs #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/actions/bot-autoassign/stale_pr_bot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ def unassign_linked_issues(self, pr):
return 0

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
Comment on lines +212 to +217
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
# 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.

if pr.state == "closed":
print(f"PR #{pr.number} is already closed, skipping")
return True
Expand Down
1 change: 1 addition & 0 deletions .github/actions/bot-autoassign/tests/test_stale_pr_bot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ def test_success(self, bot_env):
mock_issue.remove_from_assignees.assert_called_once_with("testuser")


@pytest.mark.skip(reason="Auto-close temporarily disabled; see close_stale_pr stub.")
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

class TestCloseStalePR:
def test_success(self, bot_env):
bot = StalePRBot()
Expand Down
Loading