Enforce exact-head reviewer approval as a mandatory PR merge gate #916

New Issue

2026-07-25T17:44:22-05:00

jcwalker3 commented

2026-07-25 17:44:22 -05:00

Problem

The workflow must never allow a pull request to be merged before an independent reviewer has approved its exact current head SHA.

Author completion, passing tests, a clean merge result, or a previous approval at an older head must not substitute for a current reviewer approval.

A REQUEST_CHANGES verdict must block merging until:

The author remediates the requested changes.
The updated PR head is published.
An independent reviewer performs a fresh review.
The reviewer submits APPROVE for that exact new head.

Any head-changing operation—including remediation commits, update-by-merge, conflict resolution, rebase, or other new commits—must invalidate earlier approvals for merge purposes.

Required behavior

The merger workflow must fail closed unless all of these conditions are true immediately before merge_pr:

approval_at_current_head=true
approval_valid_for_merge=true
the valid verdict for the current head is APPROVE
no active or unresolved REQUEST_CHANGES state exists
recommended_next_action=merge_now
commits_behind=0
the PR is mergeable and conflict-free
required checks are acceptable
the approved head equals both the live PR head and expected_head_sha
the approving reviewer is independent of the PR author

Reviewing, approving, and merging must remain separate workflow phases with role enforcement.

Required lifecycle

Author creates or updates the PR.
Independent reviewer reviews the exact current head.
If the verdict is REQUEST_CHANGES, merging remains blocked.
Author remediates and republishes the PR.
Any changed head requires a fresh independent review.
Only a valid APPROVE at the exact live head makes the PR merge-eligible.
Merger rechecks the approval and head pin immediately before mutation.
Any mismatch or indeterminate state denies the merge without mutation.

Acceptance criteria

A PR with no review cannot be merged.
A PR with only comments or non-approval reviews cannot be merged.
A PR with an active or unresolved REQUEST_CHANGES verdict cannot be merged.
Author remediation does not automatically clear the review gate.
An approval for an older head becomes stale after any head change.
An update-by-merge invalidates approval and requires a fresh review.
A fresh APPROVE at the exact current head restores merge eligibility.
merge_pr rejects an expected_head_sha different from the approved or live head.
Missing, malformed, unavailable, or ambiguous review state fails closed.
The author cannot approve their own PR for merge eligibility.
Workflow routing recommends remediation or fresh review instead of merge_now whenever approval is missing or stale.
Native review evidence and issue-side proof remain attributable to the reviewed head.
Existing correctly approved merge behavior remains supported.

Required tests

Add focused unit, integration, and workflow tests covering at least:

No reviews.
Comment-only review.
REQUEST_CHANGES at the current head.
Remediation commit without fresh review.
Approval at a stale head.
Update-by-merge after approval.
Fresh approval at the new head.
Approval/live-head mismatch.
Approval/expected_head_sha mismatch.
Self-approval attempt.
Multiple reviews where the latest authoritative state remains blocking.
Missing or malformed native review evidence.
Race where the head changes between assessment and merge_pr.
Successful merge at an independently approved, exact, unchanged head.

Include regression tests proving the enforcement cannot be bypassed through alternate merge entry points, lease adoption, cached sync results, or direct capability invocation.

Mutation-test the critical predicates so removing exact-head validation, ignoring REQUEST_CHANGES, accepting stale approval, or permitting missing review state causes tests to fail.

Out of scope

Automatically approving PRs.
Treating tests as a substitute for review.
Allowing authors or mergers to manufacture reviewer approval.
Weakening independent-review or role-separation rules.
Bypassing existing parity, lease, synchronization, conflict, or required-check gates.

## Problem The workflow must never allow a pull request to be merged before an independent reviewer has approved its exact current head SHA. Author completion, passing tests, a clean merge result, or a previous approval at an older head must not substitute for a current reviewer approval. A `REQUEST_CHANGES` verdict must block merging until: 1. The author remediates the requested changes. 2. The updated PR head is published. 3. An independent reviewer performs a fresh review. 4. The reviewer submits `APPROVE` for that exact new head. Any head-changing operation—including remediation commits, update-by-merge, conflict resolution, rebase, or other new commits—must invalidate earlier approvals for merge purposes. ## Required behavior The merger workflow must fail closed unless all of these conditions are true immediately before `merge_pr`: * `approval_at_current_head=true` * `approval_valid_for_merge=true` * the valid verdict for the current head is `APPROVE` * no active or unresolved `REQUEST_CHANGES` state exists * `recommended_next_action=merge_now` * `commits_behind=0` * the PR is mergeable and conflict-free * required checks are acceptable * the approved head equals both the live PR head and `expected_head_sha` * the approving reviewer is independent of the PR author Reviewing, approving, and merging must remain separate workflow phases with role enforcement. ## Required lifecycle 1. Author creates or updates the PR. 2. Independent reviewer reviews the exact current head. 3. If the verdict is `REQUEST_CHANGES`, merging remains blocked. 4. Author remediates and republishes the PR. 5. Any changed head requires a fresh independent review. 6. Only a valid `APPROVE` at the exact live head makes the PR merge-eligible. 7. Merger rechecks the approval and head pin immediately before mutation. 8. Any mismatch or indeterminate state denies the merge without mutation. ## Acceptance criteria * A PR with no review cannot be merged. * A PR with only comments or non-approval reviews cannot be merged. * A PR with an active or unresolved `REQUEST_CHANGES` verdict cannot be merged. * Author remediation does not automatically clear the review gate. * An approval for an older head becomes stale after any head change. * An update-by-merge invalidates approval and requires a fresh review. * A fresh `APPROVE` at the exact current head restores merge eligibility. * `merge_pr` rejects an `expected_head_sha` different from the approved or live head. * Missing, malformed, unavailable, or ambiguous review state fails closed. * The author cannot approve their own PR for merge eligibility. * Workflow routing recommends remediation or fresh review instead of `merge_now` whenever approval is missing or stale. * Native review evidence and issue-side proof remain attributable to the reviewed head. * Existing correctly approved merge behavior remains supported. ## Required tests Add focused unit, integration, and workflow tests covering at least: 1. No reviews. 2. Comment-only review. 3. `REQUEST_CHANGES` at the current head. 4. Remediation commit without fresh review. 5. Approval at a stale head. 6. Update-by-merge after approval. 7. Fresh approval at the new head. 8. Approval/live-head mismatch. 9. Approval/`expected_head_sha` mismatch. 10. Self-approval attempt. 11. Multiple reviews where the latest authoritative state remains blocking. 12. Missing or malformed native review evidence. 13. Race where the head changes between assessment and `merge_pr`. 14. Successful merge at an independently approved, exact, unchanged head. Include regression tests proving the enforcement cannot be bypassed through alternate merge entry points, lease adoption, cached sync results, or direct capability invocation. Mutation-test the critical predicates so removing exact-head validation, ignoring `REQUEST_CHANGES`, accepting stale approval, or permitting missing review state causes tests to fail. ## Out of scope * Automatically approving PRs. * Treating tests as a substitute for review. * Allowing authors or mergers to manufacture reviewer approval. * Weakening independent-review or role-separation rules. * Bypassing existing parity, lease, synchronization, conflict, or required-check gates.

jcwalker3 referenced this issue

2026-07-28 02:52:04 -05:00

Remote-MCP: deny-by-default server-side authorization over the full request tuple, read-only by default #957

jcwalker3 referenced this issue

2026-07-28 02:56:38 -05:00

Canary: five concurrent LLM clients with a negative concurrency suite #963

jcwalker3 referenced this issue

2026-07-28 03:02:56 -05:00

Epic: Make author workflows transactional and safe for multiple LLM clients #887

jcwalker3 referenced this issue

2026-07-28 03:07:51 -05:00

Epic: certify safe multi-LLM execution through securely isolated remote MCP #955