Skip to content

Commit 3d3524f

Browse files
authored
refactor: harden reviewer handoff command surface (#562)
* refactor: harden reviewer handoff command surface * fix: align reviewer handoff hardening * fix: close reviewer feedback handoff gaps * fix: finalize reviewer feedback handoff ordering * fix: close reviewer command authority gaps
1 parent b08bcde commit 3d3524f

28 files changed

Lines changed: 1684 additions & 140 deletions

REVIEWING.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ permission to persist reviewer-bot state.
8686

8787
Who can run it:
8888
- The currently assigned reviewer
89-
- A maintainer with triage+ permission
9089

9190
What it does (for the current PR only):
9291
- If the latest review by the assigned reviewer is `APPROVED`, it marks the review complete.

scripts/reviewer_bot_core/comment_command_policy.py

Lines changed: 21 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+
FEEDBACK = "feedback"
1415
DONE = "done"
1516
LABEL = "label"
1617
SYNC_MEMBERS = "sync-members"
@@ -103,6 +104,26 @@ def decide_comment_command(bot, request, classified, *, actor_class: str, comman
103104
success=False,
104105
react=True,
105106
)
107+
if command == OrdinaryCommandId.FEEDBACK.value:
108+
if args:
109+
return InlineResponseDecision(
110+
response=f"❌ `/feedback` does not accept arguments. Usage: `{bot.BOT_MENTION} /feedback`",
111+
success=False,
112+
react=True,
113+
)
114+
return ExecuteOrdinaryCommandDecision(
115+
command_id=OrdinaryCommandId.FEEDBACK,
116+
issue_number=issue_number,
117+
actor=comment_author,
118+
raw_args=(),
119+
needs_assignment_request=False,
120+
)
121+
if command == "_malformed_feedback_args":
122+
return InlineResponseDecision(
123+
response=f"❌ `/feedback` does not accept arguments. Usage: `{bot.BOT_MENTION} /feedback`",
124+
success=False,
125+
react=True,
126+
)
106127
if command == OrdinaryCommandId.DONE.value:
107128
return ExecuteOrdinaryCommandDecision(
108129
command_id=OrdinaryCommandId.DONE,

scripts/reviewer_bot_core/review_state_machine.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,18 @@ def _compare_records(left: dict | None, right: dict | None) -> int:
9797
return 0
9898

9999

100+
def _clear_handoff_after_newer_contributor_event(review_data: dict, timestamp: str) -> bool:
101+
handoff = review_data.get("current_cycle_reviewer_handoff")
102+
if not isinstance(handoff, dict):
103+
return False
104+
contributor_time = parse_github_timestamp(timestamp)
105+
handoff_time = parse_github_timestamp(handoff.get("timestamp"))
106+
if contributor_time is None or handoff_time is None or contributor_time <= handoff_time:
107+
return False
108+
review_data["current_cycle_reviewer_handoff"] = None
109+
return True
110+
111+
100112
def semantic_key_seen(review_data: dict, channel_name: str, semantic_key: str) -> bool:
101113
channel = _ensure_channel_map(review_data, channel_name)
102114
return semantic_key in channel["seen_keys"]
@@ -135,6 +147,8 @@ def accept_channel_event(
135147
current = channel.get("accepted")
136148
if _compare_records(candidate, current) >= 0:
137149
channel["accepted"] = candidate
150+
if channel_name in {"contributor_comment", "contributor_revision"}:
151+
_clear_handoff_after_newer_contributor_event(review_data, timestamp)
138152
return True
139153

140154

@@ -161,9 +175,17 @@ def _reset_cycle_state(review_data: dict) -> None:
161175
review_data[channel] = {"accepted": None, "seen_keys": []}
162176
review_data["current_cycle_completion"] = {}
163177
review_data["current_cycle_write_approval"] = {}
178+
review_data["current_cycle_reviewer_handoff"] = None
164179
review_data["overdue_anchor"] = None
165180

166181

182+
def clear_current_cycle_reviewer_handoff(review_data: dict) -> bool:
183+
if review_data.get("current_cycle_reviewer_handoff") is None:
184+
return False
185+
review_data["current_cycle_reviewer_handoff"] = None
186+
return True
187+
188+
167189
def set_current_reviewer(
168190
state: dict,
169191
issue_number: int,
@@ -204,6 +226,7 @@ def clear_current_reviewer(state: dict, issue_number: int) -> bool:
204226
if review_data.get("assignment_method") is not None:
205227
review_data["assignment_method"] = None
206228
changed = True
229+
changed = clear_current_cycle_reviewer_handoff(review_data) or changed
207230
clear_transition_timers(review_data)
208231
return changed
209232

scripts/reviewer_bot_core/review_state_types.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ class ReviewChannelState:
4545
seen_keys: list[str] = field(default_factory=list)
4646

4747

48+
@dataclass
49+
class CurrentCycleReviewerHandoff:
50+
"""Maps to the exact persisted `/feedback` reviewer handoff shape."""
51+
52+
source_event_key: str
53+
timestamp: str
54+
actor: str
55+
command_name: str
56+
reviewed_head_sha: str | None = None
57+
58+
4859
@dataclass
4960
class ReviewEntryState:
5061
"""Maps to the persisted review entry fields used by the future C1 cutover.
@@ -79,3 +90,4 @@ class ReviewEntryState:
7990
review_dismissal: ReviewChannelState = field(default_factory=ReviewChannelState)
8091
current_cycle_completion: dict[str, Any] = field(default_factory=dict)
8192
current_cycle_write_approval: dict[str, Any] = field(default_factory=dict)
93+
current_cycle_reviewer_handoff: CurrentCycleReviewerHandoff | None = None

0 commit comments

Comments
 (0)