Skip to content

Commit e5afa3d

Browse files
committed
fix(reviewer-bot): close reminder authority and completion gaps
Prevent stale reviewer authority, duplicate reminder spam, and missing issue-review completion handling from diverging across reminders, lifecycle events, and operator-visible projection.
1 parent 4d44d53 commit e5afa3d

40 files changed

Lines changed: 2380 additions & 331 deletions

.github/workflows/reviewer-bot-issues.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: Reviewer Bot Issues
22

33
on:
44
issues:
5-
types: [opened, labeled, edited, closed]
5+
types: [opened, edited, labeled, unlabeled, assigned, unassigned, reopened, closed]
66

77
permissions:
88
contents: read
@@ -50,6 +50,8 @@ jobs:
5050
EVENT_NAME: issues
5151
EVENT_ACTION: ${{ github.event.action }}
5252
ISSUE_NUMBER: ${{ github.event.issue.number }}
53+
IS_PULL_REQUEST: 'false'
54+
ISSUE_STATE: ${{ github.event.issue.state }}
5355
ISSUE_TITLE: ${{ github.event.issue.title }}
5456
ISSUE_BODY: ${{ github.event.issue.body }}
5557
ISSUE_UPDATED_AT: ${{ github.event.issue.updated_at }}

REVIEWING.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ If you are contributing a guideline, start with [CONTRIBUTING.md](CONTRIBUTING.m
66
## Reviewer Bot Commands
77

88
> [!NOTE]
9-
> These commands only apply in the context of coding guideline issues.
9+
> These commands apply to tracked reviewer-bot issue reviews. Some commands remain specific to coding-guideline or FLS-audit issues.
1010
1111
Before we continue, here's a preamble on how the reviewer bot helps reviewers do their job.
1212

@@ -98,6 +98,23 @@ What it does (for the current PR only):
9898
@guidelines-bot /rectify
9999
```
100100

101+
### Mark a Tracked Issue Review Complete
102+
103+
```
104+
@guidelines-bot /done
105+
```
106+
107+
Use this to mark a tracked non-PR issue review complete when the review work is finished.
108+
109+
- Available on generic tracked issues and FLS audit issues
110+
- Not available on pull requests
111+
- Not available on coding guideline issues, which still use `sign-off: create pr`
112+
113+
**Example:**
114+
```
115+
@guidelines-bot /done
116+
```
117+
101118
### Assign a Specific Reviewer
102119

103120
```

scripts/reviewer_bot_core/comment_command_policy.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
class OrdinaryCommandId(StrEnum):
1212
PASS = "pass"
1313
AWAY = "away"
14+
DONE = "done"
1415
LABEL = "label"
1516
SYNC_MEMBERS = "sync-members"
1617
QUEUE = "queue"
@@ -102,6 +103,14 @@ def decide_comment_command(bot, request, classified, *, actor_class: str, comman
102103
success=False,
103104
react=True,
104105
)
106+
if command == OrdinaryCommandId.DONE.value:
107+
return ExecuteOrdinaryCommandDecision(
108+
command_id=OrdinaryCommandId.DONE,
109+
issue_number=issue_number,
110+
actor=comment_author,
111+
raw_args=args,
112+
needs_assignment_request=True,
113+
)
105114
if command == OrdinaryCommandId.LABEL.value:
106115
return ExecuteOrdinaryCommandDecision(
107116
command_id=OrdinaryCommandId.LABEL,

scripts/reviewer_bot_core/review_state_machine.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,21 @@ def set_current_reviewer(
193193
_reset_cycle_state(review_data)
194194

195195

196+
def clear_current_reviewer(state: dict, issue_number: int) -> bool:
197+
review_data = ensure_review_entry(state, issue_number)
198+
if review_data is None:
199+
return False
200+
changed = False
201+
if review_data.get("current_reviewer") is not None:
202+
review_data["current_reviewer"] = None
203+
changed = True
204+
if review_data.get("assignment_method") is not None:
205+
review_data["assignment_method"] = None
206+
changed = True
207+
clear_transition_timers(review_data)
208+
return changed
209+
210+
196211
def update_reviewer_activity(state: dict, issue_number: int, reviewer: str, *, now: str) -> bool:
197212
review_data = ensure_review_entry(state, issue_number)
198213
if review_data is None:

scripts/reviewer_bot_core/reviewer_response_policy.py

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ def _initial_reviewer_anchor(review_data: dict) -> str | None:
5555
return None
5656

5757

58+
def _record_for_current_reviewer(record: dict | None | object, current_reviewer: str) -> dict | None:
59+
if not isinstance(record, dict):
60+
return None
61+
actor = record.get("actor")
62+
if not isinstance(actor, str) or not actor.strip():
63+
return record
64+
if actor.lower() != current_reviewer.lower():
65+
return None
66+
return record
67+
68+
5869
def _contributor_revision_handoff_record(review_data: dict, current_head: str | None, reviewer_review: dict | None) -> dict | None:
5970
contributor_revision = review_data.get("contributor_revision", {}).get("accepted")
6071
if not isinstance(contributor_revision, dict):
@@ -93,33 +104,69 @@ def derive_reviewer_response_state(
93104
contributor_comment = review_data.get("contributor_comment", {}).get("accepted")
94105

95106
if not issue_is_pull_request:
96-
if not reviewer_comment and not reviewer_review:
107+
reviewer_comment = _record_for_current_reviewer(reviewer_comment, current_reviewer)
108+
reviewer_review = _record_for_current_reviewer(reviewer_review, current_reviewer)
109+
latest_reviewer_response = reviewer_comment
110+
if reviewer_review_helpers.compare_records(
111+
reviewer_review,
112+
latest_reviewer_response,
113+
parse_timestamp=live_review_support.parse_github_timestamp,
114+
) > 0:
115+
latest_reviewer_response = reviewer_review
116+
completion = review_data.get("current_cycle_completion")
117+
if isinstance(completion, dict) and completion.get("completed"):
118+
return {
119+
"state": "done",
120+
"reason": None,
121+
"anchor_timestamp": latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None,
122+
"reviewer_comment": reviewer_comment,
123+
"reviewer_review": reviewer_review,
124+
"contributor_comment": contributor_comment,
125+
"contributor_handoff": contributor_comment,
126+
}
127+
if review_data.get("review_completed_at"):
128+
return {
129+
"state": "done",
130+
"reason": None,
131+
"anchor_timestamp": latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None,
132+
"reviewer_comment": reviewer_comment,
133+
"reviewer_review": reviewer_review,
134+
"contributor_comment": contributor_comment,
135+
"contributor_handoff": contributor_comment,
136+
}
137+
if not latest_reviewer_response:
97138
return {
98139
"state": "awaiting_reviewer_response",
99140
"reason": "no_reviewer_activity",
100141
"anchor_timestamp": _initial_reviewer_anchor(review_data),
101142
"reviewer_comment": reviewer_comment,
102143
"reviewer_review": reviewer_review,
103144
"contributor_comment": contributor_comment,
104-
"contributor_handoff": None,
145+
"contributor_handoff": contributor_comment,
105146
}
106-
latest_reviewer_response = reviewer_comment
107-
if reviewer_review_helpers.compare_records(
108-
reviewer_review,
147+
if _compare_cross_channel_conversation(
148+
contributor_comment,
109149
latest_reviewer_response,
110150
parse_timestamp=live_review_support.parse_github_timestamp,
111151
) > 0:
112-
latest_reviewer_response = reviewer_review
113-
completion = review_data.get("current_cycle_completion")
114-
if not isinstance(completion, dict) or not completion.get("completed"):
115-
if review_data.get("review_completed_at"):
116-
return {"state": "done", "reason": None}
117152
return {
118-
"state": "awaiting_contributor_response",
119-
"reason": "completion_missing",
120-
"anchor_timestamp": latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None,
153+
"state": "awaiting_reviewer_response",
154+
"reason": "contributor_comment_newer",
155+
"anchor_timestamp": contributor_comment.get("timestamp") if isinstance(contributor_comment, dict) else None,
156+
"reviewer_comment": reviewer_comment,
157+
"reviewer_review": reviewer_review,
158+
"contributor_comment": contributor_comment,
159+
"contributor_handoff": contributor_comment,
121160
}
122-
return {"state": "done", "reason": None}
161+
return {
162+
"state": "awaiting_contributor_response",
163+
"reason": "completion_missing",
164+
"anchor_timestamp": latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None,
165+
"reviewer_comment": reviewer_comment,
166+
"reviewer_review": reviewer_review,
167+
"contributor_comment": contributor_comment,
168+
"contributor_handoff": contributor_comment,
169+
}
123170

124171
if not isinstance(current_head, str) or not current_head.strip():
125172
return {"state": "projection_failed", "reason": "pull_request_head_unavailable"}

scripts/reviewer_bot_lib/app.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,17 @@ def _classify_event_intent_from_context(bot: AppEventContextRuntime, context: Ev
5151
event_action = context.event_action
5252

5353
if event_name in {"issues", "pull_request_target"}:
54-
if event_action in {"opened", "labeled", "edited", "closed", "synchronize"}:
54+
if event_action in {
55+
"opened",
56+
"edited",
57+
"labeled",
58+
"unlabeled",
59+
"assigned",
60+
"unassigned",
61+
"reopened",
62+
"closed",
63+
"synchronize",
64+
}:
5565
return bot.EVENT_INTENT_MUTATING
5666
return bot.EVENT_INTENT_NON_MUTATING_READONLY
5767

@@ -174,10 +184,18 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes
174184
if event_name == "issues":
175185
if event_action == "opened":
176186
state_changed = bot.handlers.handle_issue_or_pr_opened(state)
187+
elif event_action == "assigned":
188+
state_changed = bot.handlers.handle_assigned_event(state)
189+
elif event_action == "unassigned":
190+
state_changed = bot.handlers.handle_unassigned_event(state)
177191
elif event_action == "labeled":
178192
state_changed = bot.handlers.handle_labeled_event(state)
193+
elif event_action == "unlabeled":
194+
state_changed = bot.handlers.handle_unlabeled_event(state)
179195
elif event_action == "edited":
180196
state_changed = bot.handlers.handle_issue_edited_event(state)
197+
elif event_action == "reopened":
198+
state_changed = bot.handlers.handle_reopened_event(state)
181199
elif event_action == "closed":
182200
state_changed = bot.handlers.handle_closed_event(state)
183201

0 commit comments

Comments
 (0)