Corrective Stage 1 Final Follow-Up: close trusted-path and proof gaps (#552)

* fix(reviewer-bot): close final follow-up trusted path gaps

Keep trusted status-label and FLS guidance paths on their retained owners, preserve workflow-dispatch projection follow-up, and harden bootstrapped proof so these regressions are caught before merge.

* fix(reviewer-bot): align mandatory approver proof harness

Keep the mandatory approver equivalence fixtures exercising the retained transport-owned label path so branch CI matches the final follow-up owner model.

Author: PLeVasseur

scripts/reviewer_bot_lib/commands.py

@@ -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,
 )
@@ -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)
             )

scripts/reviewer_bot_lib/lifecycle.py

@@ -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(

scripts/reviewer_bot_lib/maintenance.py

@@ -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:

scripts/reviewer_bot_lib/reviews.py

@@ -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
@@ -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
@@ -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
@@ -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))
@@ -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
@@ -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
@@ -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}")
@@ -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
 

tests/contract/reviewer_bot/test_fake_runtime_contract.py

@@ -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)
 

tests/contract/reviewer_bot/test_reviewer_board_workflow_contracts.py

@@ -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

tests/contract/reviewer_bot/test_runtime_protocols.py

@@ -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)

tests/fixtures/fake_runtime.py

@@ -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)
@@ -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)