Skip to content

Commit 95fbf53

Browse files
authored
fix: close reviewer-bot stage1 corrective runtime gaps (#550)
* fix(reviewer-bot): close stage1 runtime corrective gaps * test(reviewer-bot): align fake seam stubs with runtime
1 parent 5c0dcb5 commit 95fbf53

31 files changed

+319
-227
lines changed

scripts/reviewer_bot_lib/bootstrap_runtime.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ def github_api_request(self, *args, **kwargs):
5353
def github_api(self, *args, **kwargs):
5454
return github_api.github_api(self._runtime_getter(), *args, **kwargs)
5555

56+
def github_graphql_request(self, *args, **kwargs):
57+
return github_api.github_graphql_request(self._runtime_getter(), *args, **kwargs)
58+
5659
def get_github_token(self):
5760
return github_api.get_github_token(self._runtime_getter())
5861

scripts/reviewer_bot_lib/runtime.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
STATE_READ_RETRY_LIMIT_ENV,
4747
STATUS_PROJECTION_EPOCH,
4848
TRANSITION_PERIOD_DAYS,
49+
AssignmentAttempt,
50+
GitHubApiResult,
4951
)
5052

5153

@@ -230,7 +232,9 @@ class ReviewerBotRuntime:
230232
BOT_MENTION = BOT_MENTION
231233
COMMANDS = COMMANDS
232234
FLS_AUDIT_LABEL = FLS_AUDIT_LABEL
235+
AssignmentAttempt = AssignmentAttempt
233236
AUTHOR_ASSOCIATION_TRUST_ALLOWLIST = AUTHOR_ASSOCIATION_TRUST_ALLOWLIST
237+
GitHubApiResult = GitHubApiResult
234238
REVIEWER_REQUEST_422_TEMPLATE = REVIEWER_REQUEST_422_TEMPLATE
235239
REVIEW_FRESHNESS_RUNBOOK_PATH = REVIEW_FRESHNESS_RUNBOOK_PATH
236240
REVIEW_DEADLINE_DAYS = REVIEW_DEADLINE_DAYS
@@ -365,6 +369,9 @@ def github_api_request(self, *args, **kwargs):
365369
def github_api(self, *args, **kwargs):
366370
return self.github.github_api(*args, **kwargs)
367371

372+
def github_graphql_request(self, *args, **kwargs):
373+
return self.adapters.github.github_graphql_request(*args, **kwargs)
374+
368375
def collect_touched_item(self, issue_number: int | None) -> None:
369376
self.touch_tracker.collect(issue_number)
370377

scripts/reviewer_bot_lib/runtime_protocols.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -487,19 +487,14 @@ class CommentApplicationRuntimeContext(Protocol):
487487
BOT_MENTION: str
488488
github: CommentGitHubWriteContext
489489

490-
def get_config_value(self, name: str, default: str = "") -> str: ...
491-
492490

493491
@runtime_checkable
494492
class CommentRoutingRuntimeContext(Protocol):
495493
BOT_NAME: str
496494
BOT_MENTION: str
497-
AUTHOR_ASSOCIATION_TRUST_ALLOWLIST: tuple[str, ...]
498495
logger: Logger
499496
adapters: CommentRoutingAdaptersContext
500497

501498
def assert_lock_held(self, context: str) -> None: ...
502499

503500
def collect_touched_item(self, issue_number: int | None) -> None: ...
504-
505-
def github_api(self, *args, **kwargs) -> Any | None: ...

tests/contract/reviewer_bot/test_adapter_contract.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from scripts import reviewer_bot
1111
from scripts.reviewer_bot_lib import event_inputs, lease_lock, review_state
12+
from scripts.reviewer_bot_lib.config import AssignmentAttempt, GitHubApiResult
1213
from scripts.reviewer_bot_lib.runtime import ReviewerBotRuntime, StdErrLogger
1314
from scripts.reviewer_bot_lib.runtime_protocols import (
1415
CommentApplicationRuntimeContext,
@@ -20,6 +21,7 @@
2021
ReconcileWorkflowRuntimeContext,
2122
)
2223
from tests.fixtures.fake_runtime import FakeReviewerBotRuntime
24+
from tests.fixtures.http_responses import FakeGitHubResponse
2325
from tests.fixtures.reviewer_bot import make_state
2426

2527

@@ -97,6 +99,29 @@ def test_runtime_head_repair_contract_is_runtime_scoped():
9799
assert hints["return"].__name__ == "HeadObservationRepairResult"
98100

99101

102+
def test_bootstrapped_runtime_re_exposes_retained_github_type_homes():
103+
runtime = reviewer_bot._runtime_bot()
104+
105+
assert runtime.AssignmentAttempt is AssignmentAttempt
106+
assert runtime.GitHubApiResult is GitHubApiResult
107+
108+
109+
def test_bootstrapped_runtime_supports_typed_rest_and_assignment_paths(monkeypatch):
110+
runtime = reviewer_bot._runtime_bot()
111+
monkeypatch.setenv("GITHUB_TOKEN", "token")
112+
runtime.rest_transport = SimpleNamespace(
113+
request=lambda *args, **kwargs: FakeGitHubResponse(201, {"ok": True}, "ok")
114+
)
115+
116+
response = runtime.github_api_request("GET", "issues/42")
117+
attempt = runtime.github.request_pr_reviewer_assignment(42, "alice")
118+
119+
assert isinstance(response, GitHubApiResult)
120+
assert response.ok is True
121+
assert isinstance(attempt, AssignmentAttempt)
122+
assert attempt.success is True
123+
124+
100125
def _protocol_member_names(protocol_type) -> set[str]:
101126
return set(protocol_type.__annotations__) | {
102127
name
@@ -546,6 +571,9 @@ def test_f2a_runtime_surface_inventory_fixture_records_retained_triples():
546571
capabilities = {entry["capability"]: entry for entry in inventory["capability_triples"]}
547572

548573
assert capabilities["comment-event dispatch"]["classification"] == "retained final surface"
574+
assert capabilities["typed REST request result"]["classification"] == "retained final surface"
575+
assert capabilities["typed assignment helper result"]["classification"] == "retained final surface"
576+
assert capabilities["reviewer-board GraphQL metadata read"]["classification"] == "retained final surface"
549577
assert "workflow-run dispatch" not in capabilities
550578
assert "refresh reviewer review from live preferred review" not in capabilities
551579
assert "repair missing reviewer review state" not in capabilities
@@ -571,6 +599,13 @@ def test_f2a_runtime_surface_inventory_matches_bootstrap_adapter_examples():
571599
assert capabilities["mandatory approver satisfaction"]["bootstrap_adapter"].endswith(
572600
"satisfy_mandatory_approver_requirement"
573601
)
602+
assert capabilities["typed REST request result"]["bootstrap_adapter"].endswith("github_api_request")
603+
assert capabilities["typed assignment helper result"]["bootstrap_adapter"].endswith(
604+
"request_pr_reviewer_assignment"
605+
)
606+
assert capabilities["reviewer-board GraphQL metadata read"]["bootstrap_adapter"].endswith(
607+
"github_graphql_request"
608+
)
574609

575610

576611
def test_f2c_no_runtime_surface_triples_are_deletion_ready_yet():

tests/contract/reviewer_bot/test_comment_application_contract.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ def test_comment_application_stores_pending_privileged_command_from_typed_reques
4747
comment_author="dana",
4848
comment_body="@guidelines-bot /accept-no-fls-changes",
4949
)
50-
harness.runtime.get_user_permission_status = lambda username, required_permission="triage": "granted"
51-
harness.runtime.post_comment = lambda issue_number, body: True
50+
harness.runtime.github.get_user_permission_status = lambda username, required_permission="triage": "granted"
51+
harness.runtime.github.post_comment = lambda issue_number, body: True
5252

5353
changed = comment_application.process_comment_event(
5454
harness.runtime,
@@ -77,8 +77,8 @@ def test_comment_application_freezes_pending_privileged_command_metadata_shape_a
7777
comment_body="@guidelines-bot /accept-no-fls-changes",
7878
)
7979
harness.runtime.set_config_value("STATE_ISSUE_NUMBER", "314")
80-
harness.runtime.get_user_permission_status = lambda username, required_permission="triage": "granted"
81-
harness.runtime.post_comment = lambda issue_number, body: posted.append((issue_number, body)) or True
80+
harness.runtime.github.get_user_permission_status = lambda username, required_permission="triage": "granted"
81+
harness.runtime.github.post_comment = lambda issue_number, body: posted.append((issue_number, body)) or True
8282

8383
changed = comment_application.process_comment_event(
8484
harness.runtime,
@@ -170,7 +170,7 @@ def test_n1_comment_application_obeys_policy_selected_handler_without_inline_com
170170
"handle_queue_command",
171171
lambda bot, current_state: calls.append((bot, current_state)) or ("", True),
172172
)
173-
harness.runtime.add_reaction = lambda *args, **kwargs: True
173+
harness.runtime.github.add_reaction = lambda *args, **kwargs: True
174174

175175
changed = comment_application.process_comment_event(
176176
harness.runtime,

tests/contract/reviewer_bot/test_fake_runtime_contract.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,21 @@ def test_fake_runtime_optional_lock_hooks_are_replaceable(monkeypatch):
155155
assert calls == ["acquire", "release"]
156156

157157

158+
def test_fake_runtime_exposes_retained_runtime_labels_and_lock_helpers(monkeypatch):
159+
runtime = FakeReviewerBotRuntime(monkeypatch)
160+
161+
assert runtime.COMMANDS
162+
assert runtime.REVIEW_LABELS
163+
assert hasattr(runtime, "github_graphql_request")
164+
assert hasattr(runtime, "normalize_lock_metadata")
165+
assert hasattr(runtime, "get_state_issue")
166+
assert hasattr(runtime, "get_state_issue_snapshot")
167+
assert hasattr(runtime, "conditional_patch_state_issue")
168+
assert hasattr(runtime, "render_state_issue_body")
169+
assert hasattr(runtime, "get_lock_ref_snapshot")
170+
assert hasattr(runtime, "renew_state_issue_lease_lock")
171+
172+
158173
def test_fake_runtime_uses_explicit_public_service_fields(monkeypatch):
159174
runtime = FakeReviewerBotRuntime(monkeypatch)
160175

@@ -322,6 +337,11 @@ def test_f2a_runtime_surface_inventory_matches_fake_runtime_branch_examples():
322337
assert capabilities["privileged accept-no-fls-changes execution"]["fake_runtime_branch"].endswith(
323338
"handle_accept_no_fls_changes_command"
324339
)
340+
assert capabilities["typed REST request result"]["fake_runtime_branch"].endswith("GitHubApiResult")
341+
assert capabilities["typed assignment helper result"]["fake_runtime_branch"].endswith("AssignmentAttempt")
342+
assert capabilities["reviewer-board GraphQL metadata read"]["fake_runtime_branch"].endswith(
343+
"github_graphql_request"
344+
)
325345
assert capabilities["mandatory approver satisfaction"]["fake_runtime_branch"].endswith(
326346
"satisfy_mandatory_approver_requirement"
327347
)

tests/contract/reviewer_bot/test_runtime_protocols.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121

2222
def _assert_core_runtime_surface(runtime) -> None:
2323
assert hasattr(runtime, "get_config_value")
24+
assert hasattr(runtime, "AssignmentAttempt")
25+
assert hasattr(runtime, "GitHubApiResult")
26+
assert hasattr(runtime, "COMMANDS")
27+
assert hasattr(runtime, "REVIEW_LABELS")
28+
assert hasattr(runtime, "github_graphql_request")
2429
assert hasattr(runtime, "infra")
2530
assert hasattr(runtime, "domain")
2631
assert runtime.infra.config is runtime.config
@@ -34,6 +39,12 @@ def _assert_core_runtime_surface(runtime) -> None:
3439
assert runtime.domain.handlers is runtime.handlers
3540

3641

42+
def test_fake_runtime_default_lock_state_matches_production_contract(monkeypatch):
43+
runtime = FakeReviewerBotRuntime(monkeypatch)
44+
45+
assert runtime.ACTIVE_LEASE_CONTEXT is None
46+
47+
3748
def test_runtime_bot_returns_concrete_runtime_object():
3849
runtime = reviewer_bot._runtime_bot()
3950

tests/fixtures/app_harness.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,18 @@ def stub_save_state(self, func) -> None:
4343
self.state_store.stub_save(func)
4444

4545
def stub_lock(self, *, acquire=None, release=None, refresh=None) -> None:
46-
self.locks.stub(acquire=acquire, release=release, refresh=refresh)
46+
def wrapped_acquire():
47+
context = acquire() if acquire is not None else object()
48+
self.runtime.ACTIVE_LEASE_CONTEXT = context if context is not None else object()
49+
return context
50+
51+
def wrapped_release():
52+
result = release() if release is not None else True
53+
if result:
54+
self.runtime.ACTIVE_LEASE_CONTEXT = None
55+
return result
56+
57+
self.locks.stub(acquire=wrapped_acquire, release=wrapped_release, refresh=refresh)
4758

4859
def stub_handler(self, name: str, func) -> None:
4960
self.handlers.stub(name, func)

tests/fixtures/commands_harness.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,25 +118,25 @@ def capture_comment_side_effects(self):
118118
return record_comment_side_effects(self.runtime)
119119

120120
def stub_assignees(self, assignees):
121-
self.runtime.get_issue_assignees = lambda issue_number: assignees
121+
self.runtime.github.get_issue_assignees = lambda issue_number: assignees
122122

123123
def stub_assignment(self, *, success: bool = True, status_code: int = 201):
124124
def attempt(issue_number, username):
125125
return AssignmentAttempt(success=success, status_code=status_code)
126126

127-
self.runtime.request_pr_reviewer_assignment = attempt
128-
self.runtime.assign_issue_assignee = attempt
127+
self.runtime.github.request_pr_reviewer_assignment = attempt
128+
self.runtime.github.assign_issue_assignee = attempt
129129

130130
def stub_permission(self, status: str) -> None:
131-
self.runtime.get_user_permission_status = lambda username, required_permission="triage": status
131+
self.runtime.github.get_user_permission_status = lambda username, required_permission="triage": status
132132

133133
def stub_handler(self, name: str, func) -> None:
134134
self.handlers.stub(name, func)
135135

136136
def automation_runner(self) -> AutomationRunner:
137137
runner = AutomationRunner()
138138
self._monkeypatch.setattr(automation_module, "run_command", runner.run)
139-
self.runtime.run_command = runner.run
139+
self.runtime.adapters.automation.run_command = runner.run
140140
return runner
141141

142142
def assignment_request(self, *, issue_number: int):

tests/fixtures/comment_routing_harness.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def __init__(self, monkeypatch):
2222
self._monkeypatch = monkeypatch
2323
self.github = RouteGitHubApi()
2424
self.runtime = FakeReviewerBotRuntime(monkeypatch, github=self.github)
25+
self.runtime.ACTIVE_LEASE_CONTEXT = object()
2526
self.config = self.runtime.config
2627
self.handlers = self.runtime.handlers
2728

0 commit comments

Comments
 (0)