Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion scripts/reviewer_bot_lib/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from .event_inputs import build_assignment_request as decode_assignment_request
from .guidance import (
get_assignment_failure_comment,
get_fls_audit_guidance,
get_issue_guidance,
get_pr_guidance,
)
Expand Down Expand Up @@ -139,7 +140,7 @@ def _apply_assignment_side_effects(
else:
labels = set(request.issue_labels)
guidance = (
bot.get_fls_audit_guidance(reviewer, request.issue_author)
get_fls_audit_guidance(reviewer, request.issue_author)
if bot.FLS_AUDIT_LABEL in labels
else get_issue_guidance(reviewer, request.issue_author)
)
Expand Down
5 changes: 3 additions & 2 deletions scripts/reviewer_bot_lib/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ def _semantic_digest(value: str) -> str:


def _write_transition_notice_marker_cutover(bot) -> None:
config_dir = Path(bot.get_config_value("OPENCODE_CONFIG_DIR") or "")
if not config_dir:
config_dir_value = bot.get_config_value("OPENCODE_CONFIG_DIR").strip()
if not config_dir_value:
return
config_dir = Path(config_dir_value)
artifact_path = config_dir / "reviewer-bot" / "maintainability-remediation" / "transition-notice-marker-cutover.json"
artifact_path.parent.mkdir(parents=True, exist_ok=True)
artifact_path.write_text(
Expand Down
5 changes: 4 additions & 1 deletion scripts/reviewer_bot_lib/maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ def handle_manual_dispatch(bot, state: dict) -> bool:
bot.collect_touched_item(issue_number)
return False
if action == "check-overdue":
return maintenance_schedule.handle_scheduled_check_result(bot, state).state_changed
result = maintenance_schedule.handle_scheduled_check_result(bot, state)
for issue_number in result.touched_items:
bot.collect_touched_item(issue_number)
return result.state_changed
if action == "execute-pending-privileged-command":
source_event_key = request.privileged_source_event_key
if not source_event_key:
Expand Down
18 changes: 12 additions & 6 deletions scripts/reviewer_bot_lib/reviews.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ def is_triage_or_higher(bot, username: str) -> bool:
def trigger_mandatory_approver_escalation(bot, state: dict, issue_number: int) -> bool:
from scripts.reviewer_bot_core import mandatory_approver_policy

from . import github_api

review_data = review_state.ensure_review_entry(state, issue_number, create=True)
if review_data is None:
return False
Expand All @@ -156,7 +158,7 @@ def trigger_mandatory_approver_escalation(bot, state: dict, issue_number: int) -
state_changed = True
if decision["attempt_label_apply"]:
try:
if bot.add_label_with_status(issue_number, MANDATORY_TRIAGE_APPROVER_LABEL):
if github_api.add_label_with_status(bot, issue_number, MANDATORY_TRIAGE_APPROVER_LABEL):
if decision["record_label_applied_at"]:
review_data["mandatory_approver_label_applied_at"] = str(decision["now"])
state_changed = True
Expand All @@ -172,6 +174,8 @@ def trigger_mandatory_approver_escalation(bot, state: dict, issue_number: int) -
def satisfy_mandatory_approver_requirement(bot, state: dict, issue_number: int, approver: str) -> bool:
from scripts.reviewer_bot_core import mandatory_approver_policy

from . import github_api

review_data = review_state.ensure_review_entry(state, issue_number, create=True)
if review_data is None:
return False
Expand All @@ -186,7 +190,7 @@ def satisfy_mandatory_approver_requirement(bot, state: dict, issue_number: int,
review_data["mandatory_approver_satisfied_by"] = str(decision["approver"])
review_data["mandatory_approver_satisfied_at"] = str(decision["now"])
try:
bot.remove_label_with_status(issue_number, MANDATORY_TRIAGE_APPROVER_LABEL)
github_api.remove_label_with_status(bot, issue_number, MANDATORY_TRIAGE_APPROVER_LABEL)
except RuntimeError as exc:
_log(bot, "warning", f"Unable to remove escalation label on #{issue_number}: {exc}", issue_number=issue_number, error=str(exc))
bot.github.post_comment(issue_number, MANDATORY_TRIAGE_SATISFIED_TEMPLATE.format(approver=approver))
Expand Down Expand Up @@ -332,6 +336,8 @@ def project_status_labels_for_item(


def sync_status_labels(bot, issue_number: int, desired_labels: set[str], actual_labels: Iterable[str]) -> bool:
from . import github_api

actual_status_labels = {label for label in actual_labels if label in STATUS_LABELS}
to_add = desired_labels - actual_status_labels
to_remove = actual_status_labels - desired_labels
Expand All @@ -342,11 +348,11 @@ def sync_status_labels(bot, issue_number: int, desired_labels: set[str], actual_
raise RuntimeError(f"Unable to ensure reviewer-bot status label exists: {label}")
changed = False
for label in sorted(to_remove):
if not bot.remove_label_with_status(issue_number, label):
if not github_api.remove_label_with_status(bot, issue_number, label):
raise RuntimeError(f"Unable to remove reviewer-bot status label '{label}' from #{issue_number}")
changed = True
for label in sorted(to_add):
if not bot.add_label_with_status(issue_number, label):
if not github_api.add_label_with_status(bot, issue_number, label):
raise RuntimeError(f"Unable to add reviewer-bot status label '{label}' to #{issue_number}")
changed = True
return changed
Expand All @@ -356,7 +362,7 @@ def sync_status_labels_for_items(bot, state: dict, issue_numbers: Iterable[int])
changed = False
for issue_number in sorted({n for n in issue_numbers if isinstance(n, int) and n > 0}):
issue_snapshot = bot.github.get_issue_or_pr_snapshot(issue_number)
desired_labels, metadata = bot.project_status_labels_for_item(issue_number, state, issue_snapshot=issue_snapshot)
desired_labels, metadata = project_status_labels_for_item(bot, issue_number, state, issue_snapshot=issue_snapshot)
if desired_labels is None:
reason = metadata.get("reason") if isinstance(metadata, dict) else "unknown"
raise RuntimeError(f"Failed to derive reviewer-bot status labels for #{issue_number}: {reason}")
Expand All @@ -370,7 +376,7 @@ def sync_status_labels_for_items(bot, state: dict, issue_numbers: Iterable[int])
name = label.get("name")
if isinstance(name, str):
actual_labels.add(name)
if bot.sync_status_labels(issue_number, desired_labels, actual_labels):
if sync_status_labels(bot, issue_number, desired_labels, actual_labels):
changed = True
return changed

Expand Down
14 changes: 14 additions & 0 deletions tests/contract/reviewer_bot/test_fake_runtime_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,20 @@ def test_fake_runtime_review_state_compatibility_surface_is_limited(monkeypatch)
assert hasattr(runtime, name) is False


def test_fake_runtime_adapter_views_do_not_alias_unrelated_retained_roles(monkeypatch):
runtime = FakeReviewerBotRuntime(monkeypatch)

assert runtime.adapters.github is runtime.github
assert runtime.adapters.review_state is not runtime.adapters.commands
assert runtime.adapters.review_state is not runtime.adapters.queue
assert hasattr(runtime.adapters.review_state, "compute_reviewer_response_state")
assert hasattr(runtime.adapters.commands, "parse_command")
assert hasattr(runtime.adapters.queue, "get_next_reviewer")
assert hasattr(runtime.adapters.state_lock, "assert_lock_held")
assert hasattr(runtime.adapters.commands, "get_next_reviewer") is False
assert hasattr(runtime.adapters.queue, "parse_command") is False


def test_fake_runtime_rejects_unknown_handler_names(monkeypatch):
runtime = FakeReviewerBotRuntime(monkeypatch)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,15 @@ def test_sweeper_repair_workflow_scopes_reviewer_board_env_to_preview_only():
"REVIEWER_BOARD_TOKEN: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.action == 'preview-reviewer-board' && secrets.REVIEWER_BOARD_TOKEN || '' }}"
in workflow_text
)


def test_sweeper_repair_workflow_exports_retained_manual_dispatch_env_contract():
data = yaml.safe_load(Path(".github/workflows/reviewer-bot-sweeper-repair.yml").read_text(encoding="utf-8"))
on_block = data.get("on", data.get(True))
action_input = on_block["workflow_dispatch"]["inputs"]["action"]
workflow_text = Path(".github/workflows/reviewer-bot-sweeper-repair.yml").read_text(encoding="utf-8")

assert "check-overdue" in action_input["options"]
assert "repair-review-status-labels" in action_input["options"]
assert "MANUAL_ACTION: ${{ github.event.inputs.action }}" in workflow_text
assert "ISSUE_NUMBER: ${{ github.event.inputs.issue_number }}" in workflow_text
9 changes: 9 additions & 0 deletions tests/contract/reviewer_bot/test_runtime_protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ def _assert_core_runtime_surface(runtime) -> None:
assert runtime.domain.locks is runtime.locks
assert runtime.domain.handlers is runtime.handlers

for helper_name in (
"project_status_labels_for_item",
"sync_status_labels",
"add_label_with_status",
"remove_label_with_status",
"get_fls_audit_guidance",
):
assert hasattr(runtime, helper_name) is False


def test_fake_runtime_default_lock_state_matches_production_contract(monkeypatch):
runtime = FakeReviewerBotRuntime(monkeypatch)
Expand Down
127 changes: 121 additions & 6 deletions tests/fixtures/fake_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,127 @@ def __init__(self, *, state_store, github, locks, handlers, workflow, adapters,
self.compat = compat


class _AdapterView:
def __init__(self, target, allowed: set[str]):
self._target = target
self._allowed = set(allowed)

def __getattr__(self, name: str):
if name not in self._allowed:
raise AttributeError(name)
return getattr(self._target, name)


class FakeRuntimeAdapterServices:
def __init__(self, runtime: "FakeReviewerBotRuntime"):
self._runtime = runtime
self.workflow = runtime.workflow
self.review = runtime.compat.review
self.review_state = runtime.compat.review
self.commands = runtime.compat.review
self.queue = runtime.compat.review
self.automation = runtime.compat.automation
self.github = runtime.github
self.workflow = _AdapterView(
runtime.workflow,
{
"process_pass_until_expirations",
"sync_members_with_queue",
"sync_status_labels_for_items",
"fetch_members",
},
)
self.review_state = _AdapterView(
runtime.compat.review,
{
"maybe_record_head_observation_repair",
"handle_transition_notice",
"ensure_review_entry",
"set_current_reviewer",
"update_reviewer_activity",
"mark_review_complete",
"is_triage_or_higher",
"trigger_mandatory_approver_escalation",
"satisfy_mandatory_approver_requirement",
"compute_reviewer_response_state",
"rebuild_pr_approval_state",
},
)
self.commands = _AdapterView(
SimpleNamespace(
handle_pass_command=runtime.compat.review.handle_pass_command,
handle_pass_until_command=runtime.compat.review.handle_pass_until_command,
handle_label_command=runtime.compat.review.handle_label_command,
handle_sync_members_command=runtime.compat.review.handle_sync_members_command,
handle_queue_command=runtime.compat.review.handle_queue_command,
handle_commands_command=runtime.compat.review.handle_commands_command,
handle_claim_command=runtime.compat.review.handle_claim_command,
handle_release_command=runtime.compat.review.handle_release_command,
handle_rectify_command=runtime.compat.review.handle_rectify_command,
handle_assign_command=runtime.compat.review.handle_assign_command,
handle_assign_from_queue_command=runtime.compat.review.handle_assign_from_queue_command,
handle_accept_no_fls_changes_command=runtime.compat.automation.handle_accept_no_fls_changes_command,
get_commands_help=runtime.compat.review.get_commands_help,
strip_code_blocks=runtime.compat.review.strip_code_blocks,
parse_command=runtime.compat.review.parse_command,
),
{
"handle_pass_command",
"handle_pass_until_command",
"handle_label_command",
"handle_sync_members_command",
"handle_queue_command",
"handle_commands_command",
"handle_claim_command",
"handle_release_command",
"handle_rectify_command",
"handle_assign_command",
"handle_assign_from_queue_command",
"handle_accept_no_fls_changes_command",
"get_commands_help",
"strip_code_blocks",
"parse_command",
},
)
self.queue = _AdapterView(
runtime.compat.review,
{
"get_next_reviewer",
"record_assignment",
"reposition_member_as_next",
},
)
self.automation = _AdapterView(
runtime.compat.automation,
{
"run_command",
"summarize_output",
"list_changed_files",
"get_default_branch",
"find_open_pr_for_branch_status",
"create_pull_request",
"fetch_members",
"handle_accept_no_fls_changes_command",
},
)
self.state_lock = _AdapterView(
runtime.compat.state_lock,
{
"assert_lock_held",
"parse_iso8601_timestamp",
"normalize_lock_metadata",
"get_state_issue",
"clear_lock_metadata",
"get_state_issue_snapshot",
"conditional_patch_state_issue",
"render_state_issue_body",
"get_state_issue_html_url",
"get_lock_ref_display",
"get_lock_ref_snapshot",
"build_lock_metadata",
"create_lock_commit",
"cas_update_lock_ref",
"lock_is_currently_valid",
"renew_state_issue_lease_lock",
"ensure_state_issue_lease_lock_fresh",
"acquire_state_issue_lease_lock",
"release_state_issue_lease_lock",
},
)

def process_pass_until_expirations(self, state: dict):
return self._runtime.workflow.process_pass_until_expirations(state)
Expand Down Expand Up @@ -278,6 +390,9 @@ def parse_iso8601_timestamp(self, value: Any):
def normalize_lock_metadata(self, lock_meta: dict | None):
return state_store_module.normalize_lock_metadata(lock_meta)

def assert_lock_held(self, context: str) -> None:
return state_store_module.assert_lock_held(self._runtime, context)

def get_state_issue(self):
return state_store_module.get_state_issue(self._runtime)

Expand Down
Loading
Loading