Refactor reviewer-bot runtime context interface

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

@@ -1,6 +1,7 @@
 import pytest
 
 from scripts import reviewer_bot
+from scripts.reviewer_bot_lib.context import ReviewerBotContext
 
 
 def make_state():
@@ -27,6 +28,10 @@ def test_reviewer_bot_exports_runtime_modules():
     assert reviewer_bot.timezone is not None
 
 
+def test_reviewer_bot_satisfies_runtime_context_protocol():
+    assert isinstance(reviewer_bot, ReviewerBotContext)
+
+
 def test_render_lock_commit_message_uses_direct_json_import():
     rendered = reviewer_bot.render_lock_commit_message({"lock_state": "unlocked"})
     assert reviewer_bot.LOCK_COMMIT_MARKER in rendered

scripts/reviewer_bot_lib/app.py

@@ -3,8 +3,10 @@
 import os
 import sys
 
+from .context import ReviewerBotContext
 
-def _revalidate_epoch(bot, expected_epoch: str | None, phase: str) -> None:
+
+def _revalidate_epoch(bot: ReviewerBotContext, expected_epoch: str | None, phase: str) -> None:
     if expected_epoch is None:
         return
     latest_state = bot.load_state(fail_on_unavailable=True)
@@ -15,7 +17,7 @@ def _revalidate_epoch(bot, expected_epoch: str | None, phase: str) -> None:
         )
 
 
-def _mark_projection_repair_needed(bot, state: dict, issue_numbers: list[int], reason: str) -> bool:
+def _mark_projection_repair_needed(bot: ReviewerBotContext, state: dict, issue_numbers: list[int], reason: str) -> bool:
     changed = False
     active_reviews = state.get("active_reviews")
     if not isinstance(active_reviews, dict):
@@ -33,7 +35,7 @@ def _mark_projection_repair_needed(bot, state: dict, issue_numbers: list[int], r
     return changed
 
 
-def classify_event_intent(bot, event_name: str, event_action: str) -> str:
+def classify_event_intent(bot: ReviewerBotContext, event_name: str, event_action: str) -> str:
     """Classify whether a run can mutate reviewer-bot state."""
     if event_name in {"issues", "pull_request_target"}:
         if event_action in {"opened", "labeled", "edited", "closed", "synchronize"}:
@@ -74,12 +76,12 @@ def classify_event_intent(bot, event_name: str, event_action: str) -> str:
     return bot.EVENT_INTENT_NON_MUTATING_READONLY
 
 
-def event_requires_lease_lock(bot, event_name: str, event_action: str) -> bool:
+def event_requires_lease_lock(bot: ReviewerBotContext, event_name: str, event_action: str) -> bool:
     """Backwards-compatible helper for tests and call sites."""
     return classify_event_intent(bot, event_name, event_action) == bot.EVENT_INTENT_MUTATING
 
 
-def main(bot):
+def main(bot: ReviewerBotContext):
     """Main entry point for the reviewer bot."""
     event_name = os.environ.get("EVENT_NAME", "")
     event_action = os.environ.get("EVENT_ACTION", "")

scripts/reviewer_bot_lib/context.py

@@ -0,0 +1,107 @@
+"""Typed runtime context for extracted reviewer-bot modules."""
+
+from __future__ import annotations
+
+from collections.abc import Iterable
+from datetime import datetime
+from typing import Any, Protocol, runtime_checkable
+
+from .config import AssignmentAttempt, GitHubApiResult, LeaseContext, StateIssueSnapshot
+
+
+@runtime_checkable
+class ReviewerBotContext(Protocol):
+    """Minimal runtime surface expected by the core extracted modules.
+
+    This protocol intentionally captures true runtime services and shared state,
+    not pure helper modules or formatting helpers that should be imported
+    directly where used.
+    """
+
+    ACTIVE_LEASE_CONTEXT: LeaseContext | None
+    LOCK_API_RETRY_LIMIT: int
+    LOCK_RETRY_BASE_SECONDS: float
+    LOCK_LEASE_TTL_SECONDS: int
+    LOCK_MAX_WAIT_SECONDS: int
+    LOCK_RENEWAL_WINDOW_SECONDS: int
+    LOCK_REF_NAME: str
+    LOCK_REF_BOOTSTRAP_BRANCH: str
+    LOCK_COMMIT_MARKER: str
+    LOCK_SCHEMA_VERSION: int
+    STATE_ISSUE_NUMBER: int
+    STATE_READ_RETRY_LIMIT: int
+    STATE_READ_RETRY_BASE_SECONDS: float
+    EVENT_INTENT_MUTATING: str
+    EVENT_INTENT_NON_MUTATING_DEFER: str
+    EVENT_INTENT_NON_MUTATING_READONLY: str
+    REVIEWER_REQUEST_422_TEMPLATE: str
+    AssignmentAttempt: type[AssignmentAttempt]
+    GitHubApiResult: type[GitHubApiResult]
+    LeaseContext: type[LeaseContext]
+    sys: Any
+    datetime: type[datetime]
+    timezone: Any
+
+    def get_github_token(self) -> str: ...
+    def github_api_request(
+        self,
+        method: str,
+        endpoint: str,
+        data: dict | None = None,
+        extra_headers: dict[str, str] | None = None,
+        *,
+        suppress_error_log: bool = False,
+    ) -> GitHubApiResult: ...
+    def github_api(self, method: str, endpoint: str, data: dict | None = None) -> Any | None: ...
+    def request_reviewer_assignment(self, issue_number: int, username: str) -> AssignmentAttempt: ...
+    def remove_assignee(self, issue_number: int, username: str) -> bool: ...
+    def remove_pr_reviewer(self, issue_number: int, username: str) -> bool: ...
+    def get_state_issue(self) -> dict | None: ...
+    def get_state_issue_snapshot(self) -> StateIssueSnapshot | None: ...
+    def conditional_patch_state_issue(self, body: str, etag: str | None = None) -> GitHubApiResult: ...
+    def parse_lock_metadata_from_issue_body(self, body: str) -> dict: ...
+    def render_state_issue_body(
+        self,
+        state: dict,
+        lock_meta: dict,
+        base_body: str | None = None,
+        *,
+        preserve_state_block: bool = False,
+    ) -> str: ...
+    def assert_lock_held(self, operation: str) -> None: ...
+    def load_state(self, *, fail_on_unavailable: bool = False) -> dict: ...
+    def save_state(self, state: dict) -> bool: ...
+    def parse_iso8601_timestamp(self, value: Any) -> datetime | None: ...
+    def normalize_lock_metadata(self, lock_meta: dict | None) -> dict: ...
+    def clear_lock_metadata(self) -> dict: ...
+    def get_lock_ref_display(self) -> str: ...
+    def get_state_issue_html_url(self) -> str: ...
+    def get_lock_ref_snapshot(self) -> tuple[str, str, dict]: ...
+    def build_lock_metadata(
+        self,
+        lock_token: str,
+        lock_owner_run_id: str,
+        lock_owner_workflow: str,
+        lock_owner_job: str,
+    ) -> dict: ...
+    def create_lock_commit(self, parent_sha: str, tree_sha: str, lock_meta: dict) -> GitHubApiResult: ...
+    def cas_update_lock_ref(self, new_sha: str) -> GitHubApiResult: ...
+    def lock_is_currently_valid(self, lock_meta: dict, now: datetime | None = None) -> bool: ...
+    def renew_state_issue_lease_lock(self, context: LeaseContext) -> bool: ...
+    def ensure_state_issue_lease_lock_fresh(self) -> bool: ...
+    def acquire_state_issue_lease_lock(self) -> LeaseContext: ...
+    def release_state_issue_lease_lock(self) -> bool: ...
+    def drain_touched_items(self) -> list[int]: ...
+    def process_pass_until_expirations(self, state: dict) -> tuple[dict, list[str]]: ...
+    def sync_members_with_queue(self, state: dict) -> tuple[dict, list[str]]: ...
+    def handle_issue_or_pr_opened(self, state: dict) -> bool: ...
+    def handle_labeled_event(self, state: dict) -> bool: ...
+    def handle_issue_edited_event(self, state: dict) -> bool: ...
+    def handle_closed_event(self, state: dict) -> bool: ...
+    def handle_pull_request_target_synchronize(self, state: dict) -> bool: ...
+    def handle_pull_request_review_event(self, state: dict) -> bool: ...
+    def handle_comment_event(self, state: dict) -> bool: ...
+    def handle_manual_dispatch(self, state: dict) -> bool: ...
+    def handle_scheduled_check(self, state: dict) -> bool: ...
+    def handle_workflow_run_event(self, state: dict) -> bool: ...
+    def sync_status_labels_for_items(self, state: dict, issue_numbers: Iterable[int]) -> bool: ...

scripts/reviewer_bot_lib/github_api.py

@@ -9,6 +9,7 @@
 import requests
 
 from .config import LOCK_API_RETRY_LIMIT, LOCK_RETRY_BASE_SECONDS, STATUS_LABEL_CONFIG
+from .context import ReviewerBotContext
 
 
 def get_github_token() -> str:
@@ -20,7 +21,7 @@ def get_github_token() -> str:
 
 
 def github_api_request(
-    bot,
+    bot: ReviewerBotContext,
     method: str,
     endpoint: str,
     data: dict | None = None,
@@ -63,7 +64,7 @@ def github_api_request(
     )
 
 
-def github_api(bot, method: str, endpoint: str, data: dict | None = None):
+def github_api(bot: ReviewerBotContext, method: str, endpoint: str, data: dict | None = None):
     response = bot.github_api_request(method, endpoint, data)
     if not response.ok:
         return None
@@ -72,27 +73,27 @@ def github_api(bot, method: str, endpoint: str, data: dict | None = None):
     return response.payload
 
 
-def post_comment(bot, issue_number: int, body: str) -> bool:
+def post_comment(bot: ReviewerBotContext, issue_number: int, body: str) -> bool:
     return bot.github_api("POST", f"issues/{issue_number}/comments", {"body": body}) is not None
 
 
-def get_repo_labels(bot) -> set[str]:
+def get_repo_labels(bot: ReviewerBotContext) -> set[str]:
     result = bot.github_api("GET", "labels?per_page=100")
     if result and isinstance(result, list):
         return {label["name"] for label in result}
     return set()
 
 
-def add_label(bot, issue_number: int, label: str) -> bool:
+def add_label(bot: ReviewerBotContext, issue_number: int, label: str) -> bool:
     return bot.github_api("POST", f"issues/{issue_number}/labels", {"labels": [label]}) is not None
 
 
-def remove_label(bot, issue_number: int, label: str) -> bool:
+def remove_label(bot: ReviewerBotContext, issue_number: int, label: str) -> bool:
     bot.github_api("DELETE", f"issues/{issue_number}/labels/{quote(label, safe='')}")
     return True
 
 
-def add_label_with_status(bot, issue_number: int, label: str) -> bool:
+def add_label_with_status(bot: ReviewerBotContext, issue_number: int, label: str) -> bool:
     response = bot.github_api_request(
         "POST",
         f"issues/{issue_number}/labels",
@@ -113,7 +114,7 @@ def add_label_with_status(bot, issue_number: int, label: str) -> bool:
     return False
 
 
-def remove_label_with_status(bot, issue_number: int, label: str) -> bool:
+def remove_label_with_status(bot: ReviewerBotContext, issue_number: int, label: str) -> bool:
     response = bot.github_api_request(
         "DELETE",
         f"issues/{issue_number}/labels/{quote(label, safe='')}",
@@ -134,7 +135,7 @@ def remove_label_with_status(bot, issue_number: int, label: str) -> bool:
 
 
 def ensure_label_exists(
-    bot,
+    bot: ReviewerBotContext,
     label: str,
     *,
     color: str | None = None,
@@ -163,7 +164,7 @@ def ensure_label_exists(
     return False
 
 
-def request_reviewer_assignment(bot, issue_number: int, username: str):
+def request_reviewer_assignment(bot: ReviewerBotContext, issue_number: int, username: str):
     is_pr = os.environ.get("IS_PULL_REQUEST", "false").lower() == "true"
     if is_pr:
         endpoint = f"pulls/{issue_number}/requested_reviewers"
@@ -213,11 +214,11 @@ def request_reviewer_assignment(bot, issue_number: int, username: str):
     return bot.AssignmentAttempt(success=False, status_code=None, exhausted_retryable_failure=True)
 
 
-def assign_reviewer(bot, issue_number: int, username: str) -> bool:
+def assign_reviewer(bot: ReviewerBotContext, issue_number: int, username: str) -> bool:
     return bot.request_reviewer_assignment(issue_number, username).success
 
 
-def get_assignment_failure_comment(bot, reviewer: str, attempt) -> str | None:
+def get_assignment_failure_comment(bot: ReviewerBotContext, reviewer: str, attempt) -> str | None:
     is_pr = os.environ.get("IS_PULL_REQUEST", "false").lower() == "true"
     if attempt.status_code == 422:
         if is_pr:
@@ -236,7 +237,7 @@ def get_assignment_failure_comment(bot, reviewer: str, attempt) -> str | None:
     return None
 
 
-def get_issue_assignees(bot, issue_number: int) -> list[str]:
+def get_issue_assignees(bot: ReviewerBotContext, issue_number: int) -> list[str]:
     is_pr = os.environ.get("IS_PULL_REQUEST", "false").lower() == "true"
     if is_pr:
         result = bot.github_api("GET", f"pulls/{issue_number}")
@@ -249,21 +250,21 @@ def get_issue_assignees(bot, issue_number: int) -> list[str]:
     return []
 
 
-def add_reaction(bot, comment_id: int, reaction: str) -> bool:
+def add_reaction(bot: ReviewerBotContext, comment_id: int, reaction: str) -> bool:
     return (
         bot.github_api("POST", f"issues/comments/{comment_id}/reactions", {"content": reaction})
         is not None
     )
 
 
-def remove_assignee(bot, issue_number: int, username: str) -> bool:
+def remove_assignee(bot: ReviewerBotContext, issue_number: int, username: str) -> bool:
     return (
         bot.github_api("DELETE", f"issues/{issue_number}/assignees", {"assignees": [username]})
         is not None
     )
 
 
-def remove_pr_reviewer(bot, issue_number: int, username: str) -> bool:
+def remove_pr_reviewer(bot: ReviewerBotContext, issue_number: int, username: str) -> bool:
     return (
         bot.github_api(
             "DELETE",
@@ -274,14 +275,14 @@ def remove_pr_reviewer(bot, issue_number: int, username: str) -> bool:
     )
 
 
-def unassign_reviewer(bot, issue_number: int, username: str) -> bool:
+def unassign_reviewer(bot: ReviewerBotContext, issue_number: int, username: str) -> bool:
     is_pr = os.environ.get("IS_PULL_REQUEST", "false").lower() == "true"
     if is_pr:
         bot.remove_pr_reviewer(issue_number, username)
     return bot.remove_assignee(issue_number, username)
 
 
-def check_user_permission(bot, username: str, required_permission: str = "triage") -> bool:
+def check_user_permission(bot: ReviewerBotContext, username: str, required_permission: str = "triage") -> bool:
     result = bot.github_api("GET", f"collaborators/{username}/permission")
     if not result:
         return False