Reviewer approval must prove prior request-changes blockers are resolved or invalid #326

Closed
opened 2026-07-07 01:49:32 -05:00 by jcwalker3 · 1 comment
Owner

A PR can have a prior REQUEST_CHANGES review on a given head SHA. If a later reviewer approves the same head SHA, the workflow must prove why the previous blocker no longer applies.

Problem

Approving an unchanged head after a request-changes review can accidentally override a valid blocker.

Required behavior

  1. Before approving, fetch current PR review feedback.
  2. If there is a prior REQUEST_CHANGES, report:
    • blocking reviewer
    • blocking review timestamp
    • blocking head SHA
    • current head SHA
    • blocker text
    • whether head changed since blocker
  3. If the head did not change, approval requires proof that:
    • the earlier blocker was incorrect, or
    • the validation environment was wrong, or
    • the issue was resolved externally in a way that does not require PR head changes
  4. If this proof is missing, do not approve. Request changes or stop.

Acceptance criteria

  • Unchanged-head approvals after request-changes require explicit override proof.
  • Reports must include blocker text and reason for override.
  • Tests cover changed head, unchanged head resolved, unchanged head unresolved, and missing feedback.

Context

Related: gitea_get_pr_review_feedback, has_blocking_change_requests, author_pushed_after_request_changes, review_feedback_stale (#167, #244). Distinct from queue-skip blocker proof (#318) and broad review state-machine hardening (#290).

A PR can have a prior `REQUEST_CHANGES` review on a given head SHA. If a later reviewer approves the same head SHA, the workflow must prove why the previous blocker no longer applies. ## Problem Approving an unchanged head after a request-changes review can accidentally override a valid blocker. ## Required behavior 1. Before approving, fetch current PR review feedback. 2. If there is a prior `REQUEST_CHANGES`, report: - blocking reviewer - blocking review timestamp - blocking head SHA - current head SHA - blocker text - whether head changed since blocker 3. If the head did not change, approval requires proof that: - the earlier blocker was incorrect, or - the validation environment was wrong, or - the issue was resolved externally in a way that does not require PR head changes 4. If this proof is missing, do not approve. Request changes or stop. ## Acceptance criteria - Unchanged-head approvals after request-changes require explicit override proof. - Reports must include blocker text and reason for override. - Tests cover changed head, unchanged head resolved, unchanged head unresolved, and missing feedback. ## Context Related: `gitea_get_pr_review_feedback`, `has_blocking_change_requests`, `author_pushed_after_request_changes`, `review_feedback_stale` (#167, #244). Distinct from queue-skip blocker proof (#318) and broad review state-machine hardening (#290).
jcwalker3 added the status:in-progress label 2026-07-07 01:51:21 -05:00
Author
Owner

Claimed. Branch: feat/issue-326-request-changes-override-proof. Worktree: branches/feat-issue-326-request-changes-override-proof.

Scope: assess_request_changes_approval_proof in review_proofs + tests per acceptance criteria.

LLM-Agent-SHA: llm-4b8e1d7a3c92

Claimed. Branch: `feat/issue-326-request-changes-override-proof`. Worktree: `branches/feat-issue-326-request-changes-override-proof`. Scope: `assess_request_changes_approval_proof` in review_proofs + tests per acceptance criteria. LLM-Agent-SHA: llm-4b8e1d7a3c92
sysadmin removed the status:in-progress label 2026-07-07 02:59:35 -05:00
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#326