fix(reviewer-bot): finish reminder authority remediation

Align reminder idempotence, reviewer authority, freshness, and repair diagnostics with confirmed live GitHub assignment truth so stale stored reviewer state cannot drive reminders, reviewer-only actions, or partial-failure recovery.

Author: PLeVasseur

scripts/reviewer_bot_core/state_adapters.py

@@ -21,6 +21,14 @@
     "review_repair",
     "head_observation_repair",
     "status_label_projection",
+    "issue_snapshot_read",
+    "warning_dedupe_read",
+    "warning_post",
+    "transition_dedupe_read",
+    "transition_post",
+    "assignment_add_write",
+    "assignment_remove_write",
+    "assignment_confirm_read",
 )
 
 _DEFERRED_GAP_MIGRATION_DROP_KEYS = {
@@ -36,9 +44,13 @@ def _migrate_repair_marker(marker: Any) -> dict[str, Any] | None:
         return None
     return {
         "kind": marker.get("kind"),
+        "phase": marker.get("phase"),
+        "status_code": marker.get("status_code"),
         "reason": marker.get("reason"),
         "failure_kind": marker.get("failure_kind"),
+        "retry_attempts": marker.get("retry_attempts"),
         "recorded_at": marker.get("recorded_at"),
+        "live_assignees": deepcopy(marker.get("live_assignees")),
     }
 
 

scripts/reviewer_bot_lib/assignment_flow.py

@@ -10,6 +10,7 @@
     get_issue_guidance,
     get_pr_guidance,
 )
+from .repair_records import clear_repair_marker, store_repair_marker
 from .review_state import (
     clear_current_reviewer,
     ensure_review_entry,
@@ -39,10 +40,75 @@ def _coerce_attempt(bot, result, *, success_status: int) -> object:
     if isinstance(result, bool):
         if result:
             return _success_attempt(bot, success_status)
-        return bot.AssignmentAttempt(success=False, status_code=None)
+        return bot.AssignmentAttempt(success=False, status_code=None, failure_kind="transport_error")
     return result
 
 
+def _store_assignment_marker(bot, review_data: dict, issue_number: int, *, phase: str, marker: dict) -> bool:
+    changed = store_repair_marker(review_data, phase, marker)
+    if changed:
+        bot.collect_touched_item(issue_number)
+    return changed
+
+
+def _clear_assignment_marker(bot, review_data: dict, issue_number: int, *, phase: str) -> bool:
+    changed = clear_repair_marker(review_data, phase)
+    if changed:
+        bot.collect_touched_item(issue_number)
+    return changed
+
+
+def _assignment_attempt_marker(bot, *, phase: str, attempt) -> dict:
+    return {
+        "kind": "reminder_transport_failure",
+        "phase": phase,
+        "status_code": attempt.status_code,
+        "failure_kind": attempt.failure_kind or "transport_error",
+        "retry_attempts": attempt.retry_attempts,
+        "recorded_at": bot.clock.now().isoformat(),
+    }
+
+
+def _assignment_authority_mismatch_marker(bot, *, live_assignees: list[str], reason: str) -> dict:
+    return {
+        "kind": "reviewer_authority_mismatch",
+        "phase": "assignment_confirm_read",
+        "status_code": None,
+        "failure_kind": "reviewer_authority_mismatch",
+        "retry_attempts": 0,
+        "recorded_at": bot.clock.now().isoformat(),
+        "reason": reason,
+        "live_assignees": list(live_assignees),
+    }
+
+
+def _hard_fail_if_permission_denied(result, *, action: str, issue_number: int) -> None:
+    if result.failure_kind in {"unauthorized", "forbidden"}:
+        raise RuntimeError(
+            f"Permission denied during {action} for #{issue_number} (status {result.status_code})."
+        )
+
+
+def _read_live_assignees(bot, state: dict, issue_number: int, *, is_pull_request: bool | None = None):
+    review_data = ensure_review_entry(state, issue_number, create=True)
+    result = bot.github.get_issue_assignees_result(issue_number, is_pull_request=is_pull_request)
+    diagnostic_changed = False
+    _hard_fail_if_permission_denied(result, action="assignee confirmation read", issue_number=issue_number)
+    if not result.ok or not isinstance(result.payload, list):
+        if isinstance(review_data, dict):
+            diagnostic_changed = _store_assignment_marker(
+                bot,
+                review_data,
+                issue_number,
+                phase="assignment_confirm_read",
+                marker=_assignment_attempt_marker(bot, phase="assignment_confirm_read", attempt=result),
+            )
+        return review_data, None, result, diagnostic_changed
+    if isinstance(review_data, dict):
+        diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_confirm_read")
+    return review_data, result.payload, result, diagnostic_changed
+
+
 def _post_assignment_guidance(bot, request, reviewer: str) -> None:
     if request.is_pull_request:
         bot.github.post_comment(request.issue_number, get_pr_guidance(reviewer, request.issue_author))
@@ -85,16 +151,42 @@ def confirm_reviewer_assignment(
     pr_head_sha: str | None = None,
 ) -> dict[str, object]:
     issue_number = request.issue_number
+    review_data = ensure_review_entry(state, issue_number, create=True)
+    stored_reviewer = review_data.get("current_reviewer") if isinstance(review_data, dict) else None
+    diagnostic_changed = False
     live_before = current_assignees
     if live_before is None:
-        live_before = bot.github.get_issue_assignees(issue_number)
+        review_data, live_before, _, marker_changed = _read_live_assignees(
+            bot,
+            state,
+            issue_number,
+            is_pull_request=request.is_pull_request,
+        )
+        diagnostic_changed = marker_changed or diagnostic_changed
     if live_before is None:
-        return {"confirmed": False, "reason": "assignees_unavailable"}
+        return {
+            "confirmed": False,
+            "reason": "assignees_unavailable",
+            "diagnostic_changed": diagnostic_changed,
+        }
     if request.issue_author and reviewer.lower() == request.issue_author.lower():
+        if isinstance(review_data, dict):
+            diagnostic_changed = _store_assignment_marker(
+                bot,
+                review_data,
+                issue_number,
+                phase="assignment_confirm_read",
+                marker=_assignment_authority_mismatch_marker(
+                    bot,
+                    live_assignees=live_before,
+                    reason="self_review_not_allowed",
+                ),
+            ) or diagnostic_changed
         return {
             "confirmed": False,
             "reason": "self_review_not_allowed",
             "current_assignees": live_before,
+            "diagnostic_changed": diagnostic_changed,
         }
     removal_attempts = {}
     live_before_normalized = _normalize_logins(live_before)
@@ -104,25 +196,55 @@ def confirm_reviewer_assignment(
         attempt = _remove_live_assignee(bot, request, issue_number, assignee)
         removal_attempts[assignee] = attempt
         if not attempt.success:
-            final_assignees = bot.github.get_issue_assignees(issue_number)
+            if isinstance(review_data, dict):
+                diagnostic_changed = _store_assignment_marker(
+                    bot,
+                    review_data,
+                    issue_number,
+                    phase="assignment_remove_write",
+                    marker=_assignment_attempt_marker(bot, phase="assignment_remove_write", attempt=attempt),
+                ) or diagnostic_changed
+            _, final_assignees, _, marker_changed = _read_live_assignees(
+                bot,
+                state,
+                issue_number,
+                is_pull_request=request.is_pull_request,
+            )
+            diagnostic_changed = marker_changed or diagnostic_changed
             return {
                 "confirmed": False,
                 "reason": "remove_failed",
                 "current_assignees": live_before,
                 "final_assignees": final_assignees,
                 "removal_attempts": removal_attempts,
+                "diagnostic_changed": diagnostic_changed,
             }
     assignment_attempt = None
     if reviewer.lower() not in live_before_normalized:
         assignment_attempt = _add_live_assignee(bot, request, issue_number, reviewer)
-    final_assignees = bot.github.get_issue_assignees(issue_number)
+        if not assignment_attempt.success and isinstance(review_data, dict):
+            diagnostic_changed = _store_assignment_marker(
+                bot,
+                review_data,
+                issue_number,
+                phase="assignment_add_write",
+                marker=_assignment_attempt_marker(bot, phase="assignment_add_write", attempt=assignment_attempt),
+            ) or diagnostic_changed
+    review_data, final_assignees, _, marker_changed = _read_live_assignees(
+        bot,
+        state,
+        issue_number,
+        is_pull_request=request.is_pull_request,
+    )
+    diagnostic_changed = marker_changed or diagnostic_changed
     if final_assignees is None:
         return {
             "confirmed": False,
             "reason": "final_assignees_unknown",
             "current_assignees": live_before,
             "assignment_attempt": assignment_attempt,
             "removal_attempts": removal_attempts,
+            "diagnostic_changed": diagnostic_changed,
         }
     final_normalized = _normalize_logins(final_assignees)
     if len(final_assignees) == 1 and final_normalized[0] == reviewer.lower():
@@ -145,17 +267,41 @@ def confirm_reviewer_assignment(
             )
         if emit_guidance:
             _post_assignment_guidance(bot, request, reviewer)
+        bot.collect_touched_item(issue_number)
+        if isinstance(review_data, dict):
+            diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_add_write") or diagnostic_changed
+            diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_remove_write") or diagnostic_changed
+            diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_confirm_read") or diagnostic_changed
         return {
             "confirmed": True,
             "reviewer": reviewer,
             "current_assignees": live_before,
             "final_assignees": final_assignees,
             "assignment_attempt": assignment_attempt or _success_attempt(bot),
             "removal_attempts": removal_attempts,
+            "diagnostic_changed": diagnostic_changed,
         }
     cleared = False
-    if len(final_assignees) != 1:
+    if len(final_assignees) != 1 or (
+        len(final_assignees) == 1
+        and isinstance(stored_reviewer, str)
+        and final_normalized[0] != stored_reviewer.lower()
+    ):
         cleared = clear_current_reviewer(state, issue_number)
+        if cleared:
+            bot.collect_touched_item(issue_number)
+    if isinstance(review_data, dict):
+        diagnostic_changed = _store_assignment_marker(
+            bot,
+            review_data,
+            issue_number,
+            phase="assignment_confirm_read",
+            marker=_assignment_authority_mismatch_marker(
+                bot,
+                live_assignees=final_assignees,
+                reason="final_assignee_mismatch",
+            ),
+        ) or diagnostic_changed
     failure_comment = None
     if assignment_attempt is not None and not assignment_attempt.success:
         failure_comment = get_assignment_failure_comment(
@@ -174,6 +320,7 @@ def confirm_reviewer_assignment(
         "removal_attempts": removal_attempts,
         "failure_comment": failure_comment,
         "cleared_current_reviewer": cleared,
+        "diagnostic_changed": diagnostic_changed,
     }
 
 
@@ -186,44 +333,100 @@ def confirm_reviewer_release(
     reposition_reviewer: bool = False,
 ) -> dict[str, object]:
     issue_number = request.issue_number
-    live_before = bot.github.get_issue_assignees(issue_number)
+    review_data = ensure_review_entry(state, issue_number, create=True)
+    stored_reviewer = review_data.get("current_reviewer") if isinstance(review_data, dict) else None
+    review_data, live_before, _, diagnostic_changed = _read_live_assignees(
+        bot,
+        state,
+        issue_number,
+        is_pull_request=request.is_pull_request,
+    )
     if live_before is None:
-        return {"confirmed": False, "reason": "assignees_unavailable"}
+        return {
+            "confirmed": False,
+            "reason": "assignees_unavailable",
+            "diagnostic_changed": diagnostic_changed,
+        }
     removal_attempt = None
     if reviewer.lower() in _normalize_logins(live_before):
         removal_attempt = _remove_live_assignee(bot, request, issue_number, reviewer)
         if not removal_attempt.success:
+            if isinstance(review_data, dict):
+                diagnostic_changed = _store_assignment_marker(
+                    bot,
+                    review_data,
+                    issue_number,
+                    phase="assignment_remove_write",
+                    marker=_assignment_attempt_marker(bot, phase="assignment_remove_write", attempt=removal_attempt),
+                ) or diagnostic_changed
             return {
                 "confirmed": False,
                 "reason": "remove_failed",
                 "current_assignees": live_before,
                 "removal_attempt": removal_attempt,
+                "diagnostic_changed": diagnostic_changed,
             }
-    final_assignees = bot.github.get_issue_assignees(issue_number)
+    review_data, final_assignees, _, marker_changed = _read_live_assignees(
+        bot,
+        state,
+        issue_number,
+        is_pull_request=request.is_pull_request,
+    )
+    diagnostic_changed = marker_changed or diagnostic_changed
     if final_assignees is None:
         return {
             "confirmed": False,
             "reason": "final_assignees_unknown",
             "current_assignees": live_before,
             "removal_attempt": removal_attempt,
+            "diagnostic_changed": diagnostic_changed,
         }
     if final_assignees:
+        cleared = False
+        if len(final_assignees) != 1 or (
+            len(final_assignees) == 1
+            and isinstance(stored_reviewer, str)
+            and final_assignees[0].lower() != stored_reviewer.lower()
+        ):
+            cleared = clear_current_reviewer(state, issue_number)
+            if cleared:
+                bot.collect_touched_item(issue_number)
+        if isinstance(review_data, dict):
+            diagnostic_changed = _store_assignment_marker(
+                bot,
+                review_data,
+                issue_number,
+                phase="assignment_confirm_read",
+                marker=_assignment_authority_mismatch_marker(
+                    bot,
+                    live_assignees=final_assignees,
+                    reason="final_assignee_mismatch",
+                ),
+            ) or diagnostic_changed
         return {
             "confirmed": False,
             "reason": "final_assignee_mismatch",
             "current_assignees": live_before,
             "final_assignees": final_assignees,
             "removal_attempt": removal_attempt,
+            "cleared_current_reviewer": cleared,
+            "diagnostic_changed": diagnostic_changed,
         }
     cleared = clear_current_reviewer(state, issue_number)
+    if cleared:
+        bot.collect_touched_item(issue_number)
     if reposition_reviewer:
         bot.adapters.queue.reposition_member_as_next(state, reviewer)
+    if isinstance(review_data, dict):
+        diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_remove_write") or diagnostic_changed
+        diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_confirm_read") or diagnostic_changed
     return {
         "confirmed": True,
         "current_assignees": live_before,
         "final_assignees": final_assignees,
         "removal_attempt": removal_attempt or _success_attempt(bot, status_code=204),
         "cleared_current_reviewer": cleared,
+        "diagnostic_changed": diagnostic_changed,
     }
 
 

scripts/reviewer_bot_lib/bootstrap_runtime.py

@@ -83,8 +83,19 @@ def remove_label(self, issue_number, label):
     def ensure_label_exists(self, label, *, color=None, description=None):
         return github_api.ensure_label_exists(self._runtime_getter(), label, color=color, description=description)
 
-    def get_issue_assignees(self, issue_number):
-        return github_api.get_issue_assignees(self._runtime_getter(), issue_number)
+    def get_issue_assignees(self, issue_number, *, is_pull_request=None):
+        return github_api.get_issue_assignees(
+            self._runtime_getter(),
+            issue_number,
+            is_pull_request=is_pull_request,
+        )
+
+    def get_issue_assignees_result(self, issue_number, *, is_pull_request=None):
+        return github_api.get_issue_assignees_result(
+            self._runtime_getter(),
+            issue_number,
+            is_pull_request=is_pull_request,
+        )
 
     def request_pr_reviewer_assignment(self, issue_number, username):
         return github_api.request_pr_reviewer_assignment(self._runtime_getter(), issue_number, username)