Skip to content

Commit f69d47d

Browse files
committed
fix: preserve projection write approval authority
1 parent 0d947b2 commit f69d47d

4 files changed

Lines changed: 92 additions & 2 deletions

File tree

scripts/reviewer_bot_core/approval_policy.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,6 @@ def compute_pr_approval_state_result(
415415
permission_evidence=permission_evidence,
416416
dismissal_evidence=None,
417417
)
418-
if write_decision.response_state == "projection_failed":
419-
return live_review_support.projection_failure_result(write_decision.diagnostic_reason or write_decision.write_approval_state)
420418
completion = {
421419
"completed": completion_decision.can_set_review_completed_at,
422420
"current_head_sha": current_head,
@@ -425,6 +423,20 @@ def compute_pr_approval_state_result(
425423
),
426424
"authority_decision": completion_decision.to_output(),
427425
}
426+
if write_decision.response_state == "projection_failed":
427+
result = live_review_support.projection_failure_result(
428+
write_decision.diagnostic_reason or write_decision.write_approval_state
429+
)
430+
result["completion"] = completion
431+
result["write_approval"] = {
432+
"has_write_approval": False,
433+
"write_approvers": [],
434+
"current_head_sha": current_head,
435+
"response_state": write_decision.response_state,
436+
"authority_decision": write_decision.to_output(),
437+
}
438+
result["current_head_sha"] = current_head
439+
return result
428440
write_approval = {
429441
"has_write_approval": write_decision.response_state == "done",
430442
"write_approvers": [write_decision.approving_reviewer] if write_decision.response_state == "done" and write_decision.approving_reviewer else [],

scripts/reviewer_bot_core/reviewer_response_policy.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,13 @@ def _decorate_response(
475475
}
476476

477477

478+
def _write_approval_authority_payload(write_approval: object) -> dict[str, object] | None:
479+
if not isinstance(write_approval, dict):
480+
return None
481+
authority = write_approval.get("authority_decision")
482+
return dict(authority) if isinstance(authority, dict) else None
483+
484+
478485
def _record_for_current_reviewer(record: dict | None | object, current_reviewer: str) -> dict | None:
479486
if not isinstance(record, dict):
480487
return None
@@ -877,14 +884,19 @@ def derive_reviewer_response_state(
877884
)
878885

879886
if not isinstance(approval_result, dict) or not approval_result.get("ok"):
887+
write_approval_authority = _write_approval_authority_payload(
888+
approval_result.get("write_approval") if isinstance(approval_result, dict) else None
889+
)
880890
return _decorate_response(
881891
state="projection_failed",
882892
reason="live_review_state_unknown",
883893
scope_fields=_current_scope_fields(review_data, current_reviewer, current_head, contributor_handoff),
894+
write_approval_authority=write_approval_authority,
884895
)
885896

886897
completion = approval_result["completion"]
887898
write_approval = approval_result["write_approval"]
899+
write_approval_authority = _write_approval_authority_payload(write_approval)
888900
if not completion.get("completed"):
889901
return _decorate_response(
890902
state="awaiting_contributor_response",
@@ -911,6 +923,7 @@ def derive_reviewer_response_state(
911923
current_cycle_reviewer_handoff=reviewer_handoff,
912924
contributor_comment=contributor_comment,
913925
contributor_handoff=contributor_handoff,
926+
write_approval_authority=write_approval_authority,
914927
)
915928
if authority_response_state == "projection_failed":
916929
authority = write_approval.get("authority_decision")
@@ -921,6 +934,7 @@ def derive_reviewer_response_state(
921934
state="projection_failed",
922935
reason=reason,
923936
scope_fields=_current_scope_fields(review_data, current_reviewer, current_head, contributor_handoff),
937+
write_approval_authority=write_approval_authority,
924938
)
925939
if not write_approval.get("has_write_approval"):
926940
return _decorate_response(
@@ -934,6 +948,7 @@ def derive_reviewer_response_state(
934948
current_cycle_reviewer_handoff=reviewer_handoff,
935949
contributor_comment=contributor_comment,
936950
contributor_handoff=contributor_handoff,
951+
write_approval_authority=write_approval_authority,
937952
)
938953
return _decorate_response(
939954
state="done",
@@ -946,6 +961,7 @@ def derive_reviewer_response_state(
946961
current_cycle_reviewer_handoff=reviewer_handoff,
947962
contributor_comment=contributor_comment,
948963
contributor_handoff=contributor_handoff,
964+
write_approval_authority=write_approval_authority,
949965
)
950966

951967

tests/unit/reviewer_bot/test_reviews_live_fetch.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,23 @@ def test_compute_reviewer_response_state_reports_permission_unavailable(monkeypa
411411

412412
assert response_state["state"] == "projection_failed"
413413
assert response_state["reason"] == "live_review_state_unknown"
414+
assert response_state["write_approval_authority"] == {
415+
"issue_number": 42,
416+
"head_sha": "head-1",
417+
"assigned_reviewer": "alice",
418+
"assigned_review_id": 10,
419+
"assigned_review_state": "APPROVED",
420+
"assigned_round_complete": True,
421+
"write_approval_state": "blocked_unavailable_authority",
422+
"write_approval_source": "github_permission_read_unavailable",
423+
"approving_reviewer": "alice",
424+
"approving_review_id": 10,
425+
"permission_source": "github_permission_read",
426+
"dismissal_supersession_status": "blocked_untrusted",
427+
"response_state": "projection_failed",
428+
"diagnostic_reason": "permission_unavailable",
429+
"can_project_final_state": False,
430+
}
414431

415432

416433
def test_trigger_mandatory_approver_escalation_sets_required_label_and_ping(monkeypatch):

tests/unit/reviewer_bot/test_reviews_projection.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,51 @@ def test_status_projection_maps_reassignment_needed_and_exposes_decision_output(
355355
assert payload["output_keys"] == sorted(payload.keys())
356356

357357

358+
def test_status_projection_preserves_write_approval_authority_decision_output():
359+
authority = {
360+
"issue_number": 264,
361+
"head_sha": "head-a",
362+
"assigned_reviewer": "iglesias",
363+
"assigned_review_id": 10,
364+
"assigned_review_state": "APPROVED",
365+
"assigned_round_complete": True,
366+
"write_approval_state": "visibly_missing_write_approval",
367+
"write_approval_source": "none_visible_after_trusted_reads",
368+
"approving_reviewer": None,
369+
"approving_review_id": None,
370+
"permission_source": "github_permission_read",
371+
"dismissal_supersession_status": "pass_not_dismissed_or_superseded",
372+
"response_state": "awaiting_write_approval",
373+
"diagnostic_reason": None,
374+
"can_project_final_state": True,
375+
}
376+
decision = to_reviewer_response_decision(
377+
{
378+
"issue_number": 264,
379+
"response_state": "awaiting_write_approval",
380+
"current_scope_key": "scope-approval",
381+
"current_scope_basis": "assigned_at",
382+
"write_approval_authority": authority,
383+
}
384+
)
385+
386+
result = reviews_projection.derive_status_label_projection(
387+
reviews_projection.StatusLabelProjectionInput(
388+
issue_number=264,
389+
issue_state="open",
390+
actual_labels=(),
391+
reviewer_response=decision,
392+
reviewer_authority_outcome="tracked_reviewer_confirmed",
393+
freshness_runtime_epoch="freshness_v15",
394+
status_projection_epoch="status_projection_v2",
395+
)
396+
)
397+
398+
decision_output = result.projection_metadata["decision_output"]
399+
assert decision_output["write_approval_authority"] == authority
400+
assert result.delta.desired_status_labels == ("status: awaiting write approval",)
401+
402+
358403
@pytest.mark.parametrize("state_name", ["projection_failed", "live_read_unavailable", "unknown"])
359404
def test_status_projection_fail_closed_states_never_clear_existing_labels(state_name):
360405
decision = to_reviewer_response_decision({"response_state": state_name})

0 commit comments

Comments
 (0)