fix(reviewer-bot): clarify /release self vs triage usage

Author: PLeVasseur

.github/reviewer-bot-tests/test_reviewer_bot.py

@@ -191,6 +191,20 @@ def test_parse_command_malformed_unknown():
     assert args == ["greetings"]
 
 
+@pytest.mark.parametrize(
+    ("guidance_builder", "builder_args"),
+    [
+        (reviewer_bot.get_issue_guidance, ("alice", "bob")),
+        (reviewer_bot.get_fls_audit_guidance, ("alice", "bob")),
+        (reviewer_bot.get_pr_guidance, ("alice", "bob")),
+    ],
+)
+def test_guidance_release_commands_are_explicit(guidance_builder, builder_args):
+    guidance = guidance_builder(*builder_args)
+    assert "@guidelines-bot /release [reason]" in guidance
+    assert "@guidelines-bot /release @username [reason]" in guidance
+
+
 def test_github_api_error_handling(monkeypatch):
     class FakeResponse:
         def __init__(self, status_code, content):
@@ -426,7 +440,10 @@ def test_handle_comment_event_commands_command(stub_api, captured_comments):
     handled = reviewer_bot.handle_comment_event(state)
     assert handled is False
     assert len(captured_comments) == 1
-    assert "Available Commands" in captured_comments[0]["body"]
+    response = captured_comments[0]["body"]
+    assert "Available Commands" in response
+    assert "@guidelines-bot /release [reason]" in response
+    assert "@guidelines-bot /release @username [reason]" in response
 
 
 def test_handle_comment_event_away_command(stub_api, captured_comments, monkeypatch):
@@ -462,6 +479,49 @@ def test_handle_comment_event_release_command_self(stub_api, captured_comments,
     assert "has released" in captured_comments[0]["body"]
 
 
+def test_handle_comment_event_release_command_self_not_current_suggests_target(
+    stub_api, captured_comments, monkeypatch
+):
+    state = make_state()
+    state["active_reviews"]["42"] = {
+        "skipped": [],
+        "current_reviewer": "alice",
+        "assignment_method": "round-robin",
+    }
+    monkeypatch.setattr(reviewer_bot, "get_issue_assignees", lambda *args, **kwargs: ["alice"])
+    os.environ["COMMENT_BODY"] = "@guidelines-bot /release"
+    os.environ["COMMENT_AUTHOR"] = "bob"
+    os.environ["ISSUE_NUMBER"] = "42"
+
+    handled = reviewer_bot.handle_comment_event(state)
+
+    assert handled is False
+    assert len(captured_comments) == 1
+    response = captured_comments[0]["body"]
+    assert "@bob is not the current reviewer" in response
+    assert "Current reviewer: @alice" in response
+    assert "@guidelines-bot /release @alice" in response
+    assert "triage+ required" in response
+
+
+def test_handle_release_command_self_not_assigned_uses_single_assignee_hint(stub_api, monkeypatch):
+    state = make_state()
+    monkeypatch.setattr(reviewer_bot, "get_issue_assignees", lambda *args, **kwargs: ["alice"])
+
+    response, success = reviewer_bot.handle_release_command(
+        state=state,
+        issue_number=42,
+        comment_author="bob",
+        args=[],
+    )
+
+    assert success is False
+    assert "@bob is not assigned to this issue/PR" in response
+    assert "Current assignee(s): @alice" in response
+    assert "@guidelines-bot /release @alice" in response
+    assert "triage+ required" in response
+
+
 def test_handle_comment_event_release_command_other_requires_permission(
     stub_api, captured_comments, monkeypatch
 ):

scripts/reviewer_bot.py

@@ -20,8 +20,12 @@
     - Assign yourself as the reviewer for this issue/PR
     - Removes any existing reviewer assignment
 
-  @guidelines-bot /release [@username] [reason]
-    - Release your assignment from this issue/PR (or someone else's with triage+ permission)
+  @guidelines-bot /release [reason]
+    - Release your own assignment from this issue/PR
+    - Leaves this issue/PR unassigned
+
+  @guidelines-bot /release @username [reason]
+    - Release someone else's assignment from this issue/PR (triage+ required)
     - Does NOT auto-assign the next reviewer (use /pass for that)
 
   @guidelines-bot /rectify
@@ -96,7 +100,7 @@
 COMMANDS = {
     "pass": "Pass this review to next in queue",
     "away": "Step away from queue until date (YYYY-MM-DD)",
-    "release": "Release assignment (yours, or @username with triage+ permission)",
+    "release": "Release your assignment (/release) or another's (/release @username, triage+)",
     "rectify": "Reconcile this issue/PR's review state from GitHub",
     "claim": "Claim this review for yourself",
     "r?": "Assign a reviewer (@username or 'producers')",
@@ -673,7 +677,8 @@ def get_issue_guidance(reviewer: str, issue_author: str) -> str:
 If you need to pass this review:
 - `{BOT_MENTION} /pass [reason]` - Pass just this issue to the next reviewer
 - `{BOT_MENTION} /away YYYY-MM-DD [reason]` - Step away from the queue until a date
-- `{BOT_MENTION} /release [@username] [reason]` - Release assignment (yours or someone else's with triage+ permission)
+- `{BOT_MENTION} /release [reason]` - Release your own assignment and leave this issue unassigned
+- `{BOT_MENTION} /release @username [reason]` - Release another reviewer's assignment (triage+ required)
 
 To assign someone else:
 - `{BOT_MENTION} /r? @username` - Assign a specific reviewer
@@ -709,7 +714,8 @@ def get_fls_audit_guidance(reviewer: str, issue_author: str) -> str:
 If you need to pass this review:
 - `{BOT_MENTION} /pass [reason]` - Pass just this issue to the next reviewer
 - `{BOT_MENTION} /away YYYY-MM-DD [reason]` - Step away from the queue until a date
-- `{BOT_MENTION} /release [@username] [reason]` - Release assignment (yours or someone else's with triage+ permission)
+- `{BOT_MENTION} /release [reason]` - Release your own assignment and leave this issue unassigned
+- `{BOT_MENTION} /release @username [reason]` - Release another reviewer's assignment (triage+ required)
 
 To assign someone else:
 - `{BOT_MENTION} /r? @username` - Assign a specific reviewer
@@ -753,7 +759,8 @@ def get_pr_guidance(reviewer: str, pr_author: str) -> str:
 If you need to pass this review:
 - `{BOT_MENTION} /pass [reason]` - Pass just this PR to the next reviewer
 - `{BOT_MENTION} /away YYYY-MM-DD [reason]` - Step away from the queue until a date
-- `{BOT_MENTION} /release [@username] [reason]` - Release assignment (yours or someone else's with triage+ permission)
+- `{BOT_MENTION} /release [reason]` - Release your own assignment and leave this PR unassigned
+- `{BOT_MENTION} /release @username [reason]` - Release another reviewer's assignment (triage+ required)
 
 To assign someone else:
 - `{BOT_MENTION} /r? @username` - Assign a specific reviewer
@@ -1437,7 +1444,8 @@ def handle_commands_command() -> tuple[str, bool]:
             f"**Pass or step away:**\n"
             f"- `{BOT_MENTION} /pass [reason]` - Pass this review to next in queue (current reviewer only)\n"
             f"- `{BOT_MENTION} /away YYYY-MM-DD [reason]` - Step away from queue until a date\n"
-            f"- `{BOT_MENTION} /release [@username] [reason]` - Release assignment (yours or someone else's with triage+ permission)\n\n"
+            f"- `{BOT_MENTION} /release [reason]` - Release your own assignment and leave this unassigned\n"
+            f"- `{BOT_MENTION} /release @username [reason]` - Release another reviewer's assignment (triage+ required)\n\n"
             f"**Assign reviewers:**\n"
             f"- `{BOT_MENTION} /r? @username` - Assign a specific reviewer\n"
             f"- `{BOT_MENTION} /r? producers` - Request the next reviewer from the queue\n"
@@ -1572,10 +1580,19 @@ def handle_release_command(state: dict, issue_number: int,
             # Trying to release self when not assigned
             if tracked_reviewer:
                 return (f"❌ @{comment_author} is not the current reviewer. "
-                        f"Current reviewer: @{tracked_reviewer}"), False
+                        f"Current reviewer: @{tracked_reviewer}\n\n"
+                        f"If you meant to release @{tracked_reviewer}, use "
+                        f"`{BOT_MENTION} /release @{tracked_reviewer}` "
+                        f"(triage+ required)."), False
             elif current_assignees:
-                return (f"❌ @{comment_author} is not assigned to this issue/PR. "
-                        f"Current assignee(s): @{', @'.join(current_assignees)}"), False
+                response = (f"❌ @{comment_author} is not assigned to this issue/PR. "
+                            f"Current assignee(s): @{', @'.join(current_assignees)}")
+                if len(current_assignees) == 1:
+                    current_assignee = current_assignees[0]
+                    response += (f"\n\nIf you meant to release @{current_assignee}, use "
+                                 f"`{BOT_MENTION} /release @{current_assignee}` "
+                                 f"(triage+ required).")
+                return response, False
             else:
                 return "❌ No reviewer is currently assigned to release.", False