Skip to content

Commit 8e0ab5c

Browse files
committed
fix: harden pr264 projection repair contract
1 parent 431b1d7 commit 8e0ab5c

12 files changed

Lines changed: 348 additions & 49 deletions

.github/workflows/reviewer-bot-sweeper-repair.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ on:
1919
type: string
2020
validation_nonce:
2121
description: Validation nonce used to correlate this repair run
22-
required: false
22+
required: true
2323
type: string
2424

2525
permissions:

scripts/reviewer_bot_core/reviewer_response_policy.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
from __future__ import annotations
1616

17-
from dataclasses import dataclass
17+
from dataclasses import dataclass, replace
1818
from datetime import datetime, timezone
1919

2020
from . import live_review_support, reviewer_review_helpers
@@ -173,11 +173,16 @@ def apply_reminder_cadence_overlay(response: ReviewerResponseDecision, cadence)
173173
if response.response_state != "awaiting_reviewer_response":
174174
return response
175175
reason = getattr(cadence, "exhaustion_reason", None) or "legacy_duplicate_reminders_exhausted"
176+
scope = (
177+
replace(response.scope, scope_basis="reminder_cadence_exhausted")
178+
if response.scope is not None
179+
else None
180+
)
176181
return ReviewerResponseDecision(
177182
response_state="reviewer_reassignment_needed",
178183
reason=reason,
179184
suppression_reason=reason,
180-
scope=response.scope,
185+
scope=scope,
181186
current_head_sha=response.current_head_sha,
182187
anchor_timestamp=response.anchor_timestamp,
183188
reviewer_authority_outcome=response.reviewer_authority_outcome,

scripts/reviewer_bot_lib/app.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,8 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes
558558
if touched_items:
559559
state = bot.state_store.load_state(fail_on_unavailable=True)
560560

561+
maintenance.emit_pending_issue314_state_health_repair_summary(bot)
562+
561563
if touched_items:
562564
if not lock_acquired:
563565
raise RuntimeError(

scripts/reviewer_bot_lib/issue314_state_health.py

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,66 @@
1919
actual_status_labels_from_snapshot,
2020
)
2121

22+
_REQUIRED_IDENTITY_FIELDS = (
23+
"validation_nonce",
24+
"evaluated_repo",
25+
"head_sha",
26+
"evaluated_ref",
27+
"workflow_path",
28+
"run_id",
29+
"run_attempt",
30+
)
31+
_SUPPORTED_ACTION_WORKFLOWS = {
32+
"preview-issue314-state-health": ".github/workflows/reviewer-bot-preview.yml",
33+
"repair-issue314-state-health": ".github/workflows/reviewer-bot-sweeper-repair.yml",
34+
}
35+
_SUPPORTED_REPAIR_RESULTS = frozenset({"already_healthy", "changed", "blocked"})
36+
37+
38+
def _missing_identity_fields(values: dict[str, object]) -> tuple[str, ...]:
39+
return tuple(
40+
sorted(
41+
name
42+
for name in _REQUIRED_IDENTITY_FIELDS
43+
if not isinstance(values.get(name), str) or not str(values.get(name)).strip()
44+
)
45+
)
46+
47+
48+
def _require_identity(values: dict[str, object], *, reason: str) -> None:
49+
missing = _missing_identity_fields(values)
50+
if missing:
51+
raise RuntimeError(f"issue314_state_health_identity_blocked:{reason}:" + ",".join(missing))
52+
53+
54+
def _require_request_identity(request: "Issue314StateHealthRepairRequest") -> None:
55+
expected_workflow = _SUPPORTED_ACTION_WORKFLOWS.get(request.repair_action)
56+
if expected_workflow is None:
57+
raise RuntimeError("issue314_state_health_request_blocked:unsupported_action")
58+
_require_identity(request.__dict__, reason="missing")
59+
if request.workflow_path != expected_workflow:
60+
raise RuntimeError("issue314_state_health_request_blocked:workflow_path")
61+
62+
63+
def _require_repair_summary_contract(summary: "Issue314StateHealthRepairSummary") -> None:
64+
_require_identity(summary.__dict__, reason="missing")
65+
if summary.target_collection_mode != "global_issue314_state_health":
66+
raise RuntimeError("issue314_state_health_repair_summary_blocked:target_collection_mode")
67+
if summary.result not in _SUPPORTED_REPAIR_RESULTS:
68+
raise RuntimeError("issue314_state_health_repair_summary_blocked:result")
69+
if summary.reviewer_facing_reminder_posts_attempted != 0:
70+
raise RuntimeError("issue314_state_health_repair_summary_blocked:reviewer_reminder_attempt")
71+
if summary.manual_issue314_edit_status != "not_attempted":
72+
raise RuntimeError("issue314_state_health_repair_summary_blocked:manual_issue314_edit")
73+
if summary.rows_blocked and summary.result != "blocked":
74+
raise RuntimeError("issue314_state_health_repair_summary_blocked:blocked_rows_success")
75+
if set(summary.status_labels_changed) - set(summary.rows_repaired):
76+
raise RuntimeError("issue314_state_health_repair_summary_blocked:final_label_only_status")
77+
if summary.result == "already_healthy" and (
78+
summary.rows_repaired or summary.rows_removed_closed or summary.status_labels_changed
79+
):
80+
raise RuntimeError("issue314_state_health_repair_summary_blocked:changed_rows_marked_healthy")
81+
2282

2383
@dataclass(frozen=True)
2484
class Issue314StateHealthClassificationInput:
@@ -121,6 +181,7 @@ class Issue314StateHealthSummary:
121181
output_keys: tuple[str, ...]
122182

123183
def to_output(self) -> dict[str, object]:
184+
_require_identity(self.__dict__, reason="preview_output")
124185
payload: dict[str, object] = {
125186
"schema_version": self.schema_version,
126187
"preview_action": self.preview_action,
@@ -186,6 +247,7 @@ class Issue314StateHealthRepairSummary:
186247
result: str
187248

188249
def to_output(self) -> dict[str, object]:
250+
_require_repair_summary_contract(self)
189251
payload: dict[str, object] = {
190252
"schema_version": self.schema_version,
191253
"repair_action": self.repair_action,
@@ -285,6 +347,7 @@ def collect_issue314_state_health_input(
285347
request: Issue314StateHealthRepairRequest | None = None,
286348
) -> Issue314StateHealthClassificationInput:
287349
request = request or _default_request(bot)
350+
_require_request_identity(request)
288351
rows = _active_rows(state)
289352
snapshots: dict[int, dict[str, object]] = {}
290353
reviewer_responses: dict[int, ReviewerResponseDecision] = {}
@@ -293,8 +356,17 @@ def collect_issue314_state_health_input(
293356
for row in rows:
294357
issue_number = int(row["issue_number"])
295358
review_data = row["review_data"]
296-
snapshot_result = bot.github.get_issue_or_pr_snapshot_result(issue_number)
297-
snapshot = snapshot_result.payload if snapshot_result.ok and isinstance(snapshot_result.payload, dict) else None
359+
try:
360+
snapshot_result = bot.github.get_issue_or_pr_snapshot_result(issue_number)
361+
except (AssertionError, AttributeError, RuntimeError):
362+
snapshot_result = None
363+
snapshot = (
364+
snapshot_result.payload
365+
if snapshot_result is not None
366+
and snapshot_result.ok
367+
and isinstance(snapshot_result.payload, dict)
368+
else None
369+
)
298370
if snapshot is not None:
299371
snapshots[issue_number] = snapshot
300372
scans[issue_number] = _comment_scan(bot, issue_number)
@@ -444,6 +516,7 @@ def _row_from_input(input: Issue314StateHealthClassificationInput, row: dict[str
444516

445517

446518
def classify_issue314_state_health(input: Issue314StateHealthClassificationInput) -> Issue314StateHealthSummary:
519+
_require_identity(input.__dict__, reason="classification_input")
447520
inventory = tuple(_row_from_input(input, row) for row in input.active_review_rows)
448521
counts: dict[str, int] = {}
449522
for row in inventory:
@@ -520,6 +593,8 @@ def run_issue314_state_health_repair(
520593
state: dict,
521594
request: Issue314StateHealthRepairRequest,
522595
) -> Issue314StateHealthRepairSummary:
596+
_require_request_identity(request)
597+
bot.assert_lock_held("run_issue314_state_health_repair")
523598
classification_input = collect_issue314_state_health_input(bot, state, request)
524599
summary = classify_issue314_state_health(classification_input)
525600
active_reviews = state.get("active_reviews") if isinstance(state, dict) else None

scripts/reviewer_bot_lib/maintenance.py

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
ScheduleHandlerResult = maintenance_schedule.ScheduleHandlerResult
2727
SCHEDULE_LIKE_MANUAL_ACTIONS = frozenset({"check-overdue"})
28+
_PENDING_ISSUE314_REPAIR_SUMMARY_ATTR = "_reviewer_bot_pending_issue314_state_health_repair_summary"
2829
_now_iso = maintenance_privileged._now_iso
2930
_finalize_schedule_result = maintenance_schedule._finalize_schedule_result
3031
_record_maintenance_repair_marker = maintenance_schedule._record_maintenance_repair_marker
@@ -234,6 +235,14 @@ def _issue314_request(bot, request) -> issue314_state_health.Issue314StateHealth
234235
)
235236

236237

238+
def emit_pending_issue314_state_health_repair_summary(bot) -> None:
239+
summary = getattr(bot, _PENDING_ISSUE314_REPAIR_SUMMARY_ATTR, None)
240+
if summary is None:
241+
return
242+
issue314_state_health.emit_issue314_state_health_repair_summary(summary)
243+
setattr(bot, _PENDING_ISSUE314_REPAIR_SUMMARY_ATTR, None)
244+
245+
237246
def derive_manual_dispatch_projection_policy(request) -> ManualDispatchProjectionPolicy:
238247
action = request.action or ""
239248
issue_number = request.issue_number if isinstance(request.issue_number, int) else None
@@ -296,49 +305,34 @@ def run_status_label_repair(bot, state: dict, request: StatusLabelRepairRequest)
296305
after: list[StatusLabelRepairItemResult] = []
297306
labels_added: set[str] = set()
298307
labels_removed: set[str] = set()
299-
blocked = False
300308
for issue_number in targets:
301-
try:
302-
projection = reviews.project_status_label_projection_for_item(bot, issue_number, state)
303-
delta = projection.delta
304-
sync_result = reviews.apply_status_label_delta(bot, issue_number, delta)
305-
item_result = "changed" if sync_result.changed else "already_aligned"
306-
before.append(
307-
StatusLabelRepairItemResult(
308-
issue_number=issue_number,
309-
before_status_labels=delta.actual_status_labels,
310-
desired_status_labels=delta.desired_status_labels,
311-
labels_added=delta.labels_to_add,
312-
labels_removed=delta.labels_to_remove,
313-
result=item_result,
314-
)
315-
)
316-
after.append(
317-
StatusLabelRepairItemResult(
318-
issue_number=issue_number,
319-
before_status_labels=delta.desired_status_labels,
320-
desired_status_labels=delta.desired_status_labels,
321-
labels_added=sync_result.labels_added,
322-
labels_removed=sync_result.labels_removed,
323-
result=item_result,
324-
)
309+
projection = reviews.project_status_label_projection_for_item(bot, issue_number, state)
310+
delta = projection.delta
311+
sync_result = reviews.apply_status_label_delta(bot, issue_number, delta)
312+
item_result = "changed" if sync_result.changed else "already_aligned"
313+
before.append(
314+
StatusLabelRepairItemResult(
315+
issue_number=issue_number,
316+
before_status_labels=delta.actual_status_labels,
317+
desired_status_labels=delta.desired_status_labels,
318+
labels_added=delta.labels_to_add,
319+
labels_removed=delta.labels_to_remove,
320+
result=item_result,
325321
)
326-
labels_added.update(sync_result.labels_added)
327-
labels_removed.update(sync_result.labels_removed)
328-
except RuntimeError:
329-
blocked = True
330-
before.append(
331-
StatusLabelRepairItemResult(
332-
issue_number=issue_number,
333-
before_status_labels=(),
334-
desired_status_labels=(),
335-
labels_added=(),
336-
labels_removed=(),
337-
result="blocked",
338-
)
322+
)
323+
after.append(
324+
StatusLabelRepairItemResult(
325+
issue_number=issue_number,
326+
before_status_labels=delta.desired_status_labels,
327+
desired_status_labels=delta.desired_status_labels,
328+
labels_added=sync_result.labels_added,
329+
labels_removed=sync_result.labels_removed,
330+
result=item_result,
339331
)
340-
after.append(before[-1])
341-
result = "blocked" if blocked else "changed" if labels_added or labels_removed else "already_aligned"
332+
)
333+
labels_added.update(sync_result.labels_added)
334+
labels_removed.update(sync_result.labels_removed)
335+
result = "changed" if labels_added or labels_removed else "already_aligned"
342336
artifact = _repair_artifact_fields(request)
343337
payload = StatusLabelRepairSummary(
344338
schema_version=1,
@@ -499,7 +493,7 @@ def _handle_manual_dispatch_request(bot, state: dict, request) -> bool:
499493
state,
500494
_issue314_request(bot, request),
501495
)
502-
issue314_state_health.emit_issue314_state_health_repair_summary(summary)
496+
setattr(bot, _PENDING_ISSUE314_REPAIR_SUMMARY_ATTR, summary)
503497
return bool(summary.rows_removed_closed)
504498
if action == "execute-pending-privileged-command":
505499
source_event_key = request.privileged_source_event_key

tests/contract/reviewer_bot/test_workflow_files.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ def test_sweeper_repair_workflow_exposes_nonce_identity_and_repair_artifacts():
446446
"repair-issue314-state-health",
447447
]
448448
assert inputs["validation_nonce"]["type"] == "string"
449+
assert inputs["validation_nonce"]["required"] is True
449450
assert "VALIDATION_NONCE: ${{ github.event.inputs.validation_nonce }}" in text
450451
assert "EVALUATED_REPO: ${{ github.repository }}" in text
451452
assert "HEAD_SHA: ${{ github.sha }}" in text

tests/integration/reviewer_bot/test_app_preview_issue314_state_health.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,58 @@ def test_execute_run_preview_issue314_state_health_is_read_only_and_inspects_act
122122
assert payload["artifact_name"] == "reviewer-bot-preview-output-999-attempt-4"
123123
assert payload["artifact_file"] == "preview-output.json"
124124
assert payload["output_keys"] == sorted(payload.keys())
125+
126+
127+
def test_execute_run_preview_issue314_state_health_blocks_unavailable_live_rows(
128+
monkeypatch,
129+
capsys,
130+
):
131+
harness = AppHarness(monkeypatch)
132+
harness.set_event(
133+
EVENT_NAME="workflow_dispatch",
134+
EVENT_ACTION="",
135+
MANUAL_ACTION="preview-issue314-state-health",
136+
ISSUE_NUMBER=314,
137+
VALIDATION_NONCE="nonce-issue314-preview",
138+
GITHUB_SHA="workflow-head",
139+
GITHUB_REPOSITORY="rustfoundation/safety-critical-rust-coding-guidelines",
140+
GITHUB_RUN_ID="1000",
141+
GITHUB_RUN_ATTEMPT="1",
142+
STATE_ISSUE_NUMBER=314,
143+
)
144+
state = make_state()
145+
make_tracked_review_state(
146+
state,
147+
264,
148+
reviewer="iglesias",
149+
assigned_at="2026-02-10T17:20:07Z",
150+
active_cycle_started_at="2026-02-10T17:20:07Z",
151+
)
152+
routes = RouteGitHubApi().add_request(
153+
"GET",
154+
"issues/264",
155+
status_code=502,
156+
payload={"message": "bad gateway"},
157+
).add_request(
158+
"GET",
159+
"issues/264/comments?per_page=100&page=1",
160+
status_code=200,
161+
payload=[],
162+
)
163+
harness.runtime.github.stub(routes)
164+
harness.stub_load_state(lambda *, fail_on_unavailable=False: state)
165+
harness.stub_lock(acquire=lambda: (_ for _ in ()).throw(AssertionError("preview should not acquire lock")))
166+
harness.stub_save_state(lambda current: (_ for _ in ()).throw(AssertionError("preview should not save state")))
167+
harness.stub_sync_status_labels(lambda current, issue_numbers: (_ for _ in ()).throw(AssertionError("preview should not sync labels")))
168+
169+
result = harness.run_execute()
170+
171+
assert result.exit_code == 0
172+
payload = json.loads(capsys.readouterr().out)
173+
assert payload["active_rows_inspected"] == [264]
174+
assert payload["rows_blocked"] == [264]
175+
assert payload["row_inventory"][0]["blockers"] == [
176+
"live_snapshot_unavailable",
177+
"reviewer_response_unavailable",
178+
"status_projection_unavailable",
179+
]

tests/integration/reviewer_bot/test_app_preview_overdue.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def test_execute_run_preview_check_overdue_uses_frozen_pr264_operational_project
114114
assert payload["reviewer_authority_outcome"] == "tracked_reviewer_confirmed"
115115
assert payload["suppression_reason"] == "legacy_duplicate_reminders_exhausted"
116116
assert payload["current_scope_key"] == "reviewer=iglesias|head=head-live|cycle=2026-02-10T17:20:07Z|anchor=2026-02-10T17:20:07Z"
117-
assert payload["current_scope_basis"] == "active_cycle_started_at"
117+
assert payload["current_scope_basis"] == "reminder_cadence_exhausted"
118118
assert payload["would_post_warning"] is False
119119
assert payload["would_post_transition"] is False
120120
assert payload["lock_attempted"] is False

tests/integration/reviewer_bot/test_app_preview_status_label_projection.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def test_execute_run_preview_status_label_projection_is_read_only_pr264_contract
107107
assert payload["response_state"] == "reviewer_reassignment_needed"
108108
assert payload["reviewer_authority_outcome"] == "tracked_reviewer_confirmed"
109109
assert payload["suppression_reason"] == "legacy_duplicate_reminders_exhausted"
110-
assert payload["current_scope_basis"] == "active_cycle_started_at"
110+
assert payload["current_scope_basis"] == "reminder_cadence_exhausted"
111111
assert payload["actual_status_labels"] == ["status: awaiting reviewer response"]
112112
assert payload["desired_status_labels"] == ["status: reviewer reassignment needed"]
113113
assert payload["labels_to_add"] == ["status: reviewer reassignment needed"]

0 commit comments

Comments
 (0)