diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec84c36f..bcd7e63e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ ## [Unreleased] +- PR governance now distinguishes current-head requested changes from stale + aggregate review state before protected merge evaluation. - 긴 이메일·첨부 본문을 의미 단위 청크로 임베딩한 뒤 기존 email/attachment 벡터 계약으로 평균화하고, 청크 요청·벡터 누적을 제한된 창으로 처리합니다. OpenAI `text-embedding-3-*`에는 저장 차원(`1536`)을 직접 요청하도록 보강했습니다. 합성 메일 fixture 5건(70청크)과 provider 요청 계약으로 1,536차원 벡터 경로를 검증했으며, 실행 시 선택한 임베딩 제공자에 본문·파싱된 첨부 텍스트를 전송할 수 있습니다. 회사 기밀 데이터는 fixture·commit·PR·log에 포함하지 않습니다. - EmailDetail 테스트가 지원하지 않는 스레드 병합/분리 버튼을 `textContent`뿐 아니라 `aria-label`과 `title` 접근 가능 이름으로도 검출하도록 바꿔, 아이콘 전용 버튼 회귀를 놓치지 않습니다. diff --git a/docs/adr/0006-current-head-review-evidence.md b/docs/adr/0006-current-head-review-evidence.md new file mode 100644 index 000000000..6b89eb22e --- /dev/null +++ b/docs/adr/0006-current-head-review-evidence.md @@ -0,0 +1,54 @@ +# ADR-0006: Let exact-head robot evidence supersede stale aggregate review state + +- Status: Accepted +- Date: 2026-08-20 +- Decision owners: Naruon maintainers + +## Context + +GitHub exposes `reviewDecision=CHANGES_REQUESTED` as an aggregate pull-request +field. The field can remain `CHANGES_REQUESTED` after the requesting review was +submitted against an older commit and a later exact-head CodeRabbit or +structured OpenCode review has passed. Treating that stale aggregate as a +current blocker stranded protected merges even when current review threads and +required checks were clean. + +The GitHub REST review response includes each review's `state` and `commit_id`, +so the gate can distinguish a current request from a request attached to an +older head (GitHub, 2026). + +## Decision + +The metadata-only gate will: + +1. Read all pull-request review metadata when the aggregate decision is + `CHANGES_REQUESTED`. +2. For each reviewer, consider only the latest submitted review. Keep the + blocker when that effective review is `CHANGES_REQUESTED` on the current + head, when review metadata cannot be read, or when current-head robot review + evidence is absent or pending. +3. Treat the aggregate decision as superseded only when all requested reviews + target older commits and current-head CodeRabbit or structured OpenCode + evidence passes. +4. Never dismiss reviews, rewrite review state, use an administrator merge, or + bypass required checks. + +## Consequences + +Protected merges can proceed after a later exact-head review supersedes stale +aggregate state. A current requested change remains a hard blocker, and an API +failure fails closed. The gate performs an additional read-only reviews API +call only for pull requests whose aggregate decision is `CHANGES_REQUESTED`; +the CodeRabbit-absent path may already have read the same endpoint while +checking for structured OpenCode approval. + +## Verification + +`bash scripts/ci/test_pr_governance_gate.sh` proves stale-review supersession, +blocking of current-head requested changes, and supersession by a later +same-head approval from the same reviewer. + +## Reference + +GitHub. (2026). *REST API endpoints for pull request reviews*. GitHub Docs. +https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28 diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index 5875d21a6..68515b155 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -46,8 +46,12 @@ awaited by default. resolution via GITHUB_PATH. - Authoritative `Review skipped` evidence counts only when the same check output carries no blocking warning/failure language alongside it. -- `reviewDecision=CHANGES_REQUESTED` is a blocker until requested changes are - addressed or superseded on the current head. +- `reviewDecision=CHANGES_REQUESTED` is a blocker when a `CHANGES_REQUESTED` + review targets the current head, review metadata cannot be read, or no + current-head robot evidence supersedes the stale aggregate decision. When + every requested review targets an older commit and current-head CodeRabbit + or structured OpenCode evidence passes, the stale aggregate is informational; + the gate never dismisses the review object or bypasses a current request. - Blocker comments use the idempotent `` marker and are patched in place instead of duplicated on repeated workflow events. diff --git a/docs/doctoring/pr-governance-current-head-review-evidence.md b/docs/doctoring/pr-governance-current-head-review-evidence.md new file mode 100644 index 000000000..55cf51fe0 --- /dev/null +++ b/docs/doctoring/pr-governance-current-head-review-evidence.md @@ -0,0 +1,17 @@ +# Current-head review evidence and stale aggregate state + +Naruon's protected merge gate evaluates evidence for one immutable pull-request +head. GitHub's aggregate `reviewDecision` can remain `CHANGES_REQUESTED` after +the review was submitted on an older commit. The gate therefore reads the +review-level `commit_id`: a current-head request blocks, while only stale +requests may be superseded by passing exact-head robot evidence. + +This preserves the operational action for the customer: fix the request when it +targets the current code; otherwise continue the protected merge loop after the +new head's Checks and robot evidence pass. The gate does not dismiss reviews or +use an administrative bypass. + +## APA 7 reference + +GitHub. (2026). *REST API endpoints for pull request reviews*. GitHub Docs. +https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28 diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index a66142ca8..e057e768c 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -273,10 +273,6 @@ if [ "$MERGE_STATE" = "UNKNOWN" ]; then add_waiting "Merge state is still UNKNOWN after 4 attempts on ${HEAD_REF_OID}; waiting for GitHub to refresh mergeability." fi -if [ "$REVIEW_DECISION" = "CHANGES_REQUESTED" ]; then - add_blocker 'Review decision is CHANGES_REQUESTED; address requested changes before merge.' -fi - # shellcheck disable=SC2016 # GraphQL variables must remain literal. THREADS_JSON="$(gh api graphql \ -F owner="$OWNER" \ @@ -343,6 +339,7 @@ CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failu CODERABBIT_NO_ACTIONABLE_PATTERN='no actionable comments? (were )?generated' CHECK_RUNS="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/check-runs?per_page=100")" COMMIT_STATUS_JSON='{"statuses":[]}' +CURRENT_ROBOT_REVIEW_READY=false if ! COMMIT_STATUS_JSON="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/status" 2>"$COMMIT_STATUS_ERROR_FILE")"; then printf 'commit status lookup failed:\n' printf '%s\n' "$(<"$COMMIT_STATUS_ERROR_FILE")" | sed 's/^/ /' @@ -386,6 +383,7 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then if [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for current-head CodeRabbit evidence or a structured OpenCode App adversarial approval on ${HEAD_REF_OID}." else + CURRENT_ROBOT_REVIEW_READY=true printf 'CodeRabbit check is absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" fi fi @@ -423,6 +421,8 @@ else : elif [ "$CODERABBIT_PENDING" != "0" ] || [ "$CODERABBIT_STATUS_PENDING" != "0" ]; then add_waiting "Waiting for current-head CodeRabbit evidence on ${HEAD_REF_OID}." + else + CURRENT_ROBOT_REVIEW_READY=true fi fi @@ -476,6 +476,41 @@ else fi fi +# GitHub's aggregate reviewDecision remains CHANGES_REQUESTED after the +# requested review's commit becomes stale. Treat that aggregate as blocking +# only when a current-head request still exists, review metadata is unavailable, +# or no current-head robot evidence supersedes the stale decision. This keeps +# the gate aligned with its exact-head evidence contract without dismissing any +# review object or bypassing a current request. +if [ "$REVIEW_DECISION" = "CHANGES_REQUESTED" ]; then + if ! REVIEW_METADATA_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" 2>"$OPENCODE_REVIEWS_ERROR_FILE")"; then + printf 'Review metadata lookup failed:\n' + printf '%s\n' "$(<"$OPENCODE_REVIEWS_ERROR_FILE")" | sed 's/^/ /' + add_blocker 'Review decision metadata could not be read; see the workflow run log.' + else + CURRENT_CHANGES_REQUESTED_COUNT="$(printf '%s' "$REVIEW_METADATA_JSON" | jq -s --arg head_sha "$HEAD_SHA" ' + [ .[] + | .. + | objects + | select(has("state") and has("commit_id")) + ] + | sort_by([(.submitted_at // .created_at // ""), (.id // 0)]) + | group_by(.user.login // .user.id // "") + | map(last) + | map(select((.state // "" | ascii_upcase) == "CHANGES_REQUESTED")) + | map(select((.commit_id // "") == $head_sha)) + | length + ')" + if [ "$CURRENT_CHANGES_REQUESTED_COUNT" != "0" ]; then + add_blocker 'Review decision is CHANGES_REQUESTED; address current-head requested changes before merge.' + elif [ "$CURRENT_ROBOT_REVIEW_READY" != true ]; then + add_blocker 'Review decision remains CHANGES_REQUESTED; await current-head robot review evidence before merge.' + else + printf 'Stale CHANGES_REQUESTED review decision is superseded by current-head robot evidence on %s.\n' "$HEAD_REF_OID" + fi + fi +fi + if ! pr_snapshot_is_current; then exit 0 fi diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 8fb42aa9b..fdf4dbcd5 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -3,6 +3,9 @@ set -euo pipefail repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" script="$repo_root/scripts/ci/pr_governance_gate.sh" +test_temp_root="$(mktemp -d)" +export TMPDIR="$test_temp_root" +trap 'rm -rf -- "$test_temp_root"' EXIT make_fake_gh() { local bin_dir="$1" @@ -17,7 +20,7 @@ args="$*" if [ "$1" = "pr" ] && [ "$2" = "view" ]; then case "${GH_SCENARIO:-pass}" in - changes_requested) + changes_requested|changes_requested_current_coderabbit|changes_requested_current_review|changes_requested_current_review_superseded|missing_coderabbit_with_adversarial_approval|missing_coderabbit_with_adversarial_approval_stale) printf '{"number":42,"state":"OPEN","headRefOid":"%s","isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","reviewDecision":"CHANGES_REQUESTED","statusCheckRollup":[]}' "$head_sha" ;; transient_unknown) @@ -125,6 +128,9 @@ if [ "$1" = "pr" ] && [ "$2" = "checks" ]; then failure|failed_existing) printf '[{"name":"Application CI","state":"FAILURE","link":"https://checks/app-ci"}]' ;; + changes_requested_current_coderabbit|changes_requested_current_review|changes_requested_current_review_superseded|missing_coderabbit_with_adversarial_approval_stale) + printf '[{"name":"Application CI","state":"SUCCESS","link":"https://checks/app-ci"}]' + ;; self_gate_failed) printf '[{"name":"metadata-only gate evaluation","state":"FAILURE","link":"https://checks/governance"},{"name":"Application CI","state":"SUCCESS","link":"https://checks/app-ci"}]' ;; @@ -140,7 +146,7 @@ if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then coderabbit_pending) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"in_progress","conclusion":null,"html_url":"https://checks/coderabbit"}]}' ;; - missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) + changes_requested|changes_requested_current_review_superseded|missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_with_adversarial_approval_stale|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) printf '{"check_runs":[]}' ;; coderabbit_failed) @@ -152,6 +158,9 @@ if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then coderabbit_review_skipped) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"completed","conclusion":"neutral","output":{"title":"CodeRabbit","summary":"Review skipped","text":"Review skipped"},"html_url":"https://checks/coderabbit"}]}' ;; + changes_requested_current_coderabbit|changes_requested_current_review) + printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"completed","conclusion":"success","html_url":"https://checks/coderabbit"}]}' + ;; coderabbit_skip_with_warning) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"completed","conclusion":"neutral","output":{"title":"CodeRabbit","summary":"Review skipped","text":"Pre-merge blocking warning"},"html_url":"https://checks/coderabbit"}]}' ;; @@ -192,9 +201,21 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/pulls/42/reviews* ]]; then exit 1 fi case "${GH_SCENARIO:-pass}" in + changes_requested_current_coderabbit) + printf '[{"user":{"login":"human-reviewer"},"state":"CHANGES_REQUESTED","commit_id":"old-head"}]' + ;; + changes_requested_current_review) + printf '[{"user":{"login":"human-reviewer"},"state":"CHANGES_REQUESTED","commit_id":"%s"}]' "$head_sha" + ;; + changes_requested_current_review_superseded) + printf '[[{"user":{"login":"human-reviewer"},"state":"CHANGES_REQUESTED","commit_id":"%s","submitted_at":"2026-08-21T00:00:00Z","id":1},{"user":{"login":"human-reviewer"},"state":"APPROVED","commit_id":"%s","submitted_at":"2026-08-21T00:01:00Z","id":2}]]' "$head_sha" "$head_sha" + ;; missing_coderabbit_with_adversarial_approval) printf '[[{"user":{"login":"opencode-agent[bot]"},"state":"APPROVED","commit_id":"%s","body":"## Adversarial validation\\n\\n```json\\n{\\\"status\\\":\\\"passed\\\",\\\"probes\\\":[{\\\"outcome\\\":\\\"falsified\\\"},{\\\"outcome\\\":\\\"falsified\\\"}]}\\n```\\n\\nHead SHA: `%s`"}]]' "$head_sha" "$head_sha" ;; + missing_coderabbit_with_adversarial_approval_stale) + jq -cn --arg sha "$head_sha" '[[{"user":{"login":"human-reviewer"},"state":"CHANGES_REQUESTED","commit_id":"old-head"},{"user":{"login":"opencode-agent[bot]"},"state":"APPROVED","commit_id":$sha,"body":("## Adversarial validation\n\n```json\n{\"status\":\"passed\",\"probes\":[{\"outcome\":\"falsified\"},{\"outcome\":\"falsified\"}]}\n```\n\nHead SHA: `"+$sha+"`")}]]' + ;; missing_coderabbit_stale_approval) printf '[[{"user":{"login":"opencode-agent[bot]"},"state":"APPROVED","commit_id":"%s","body":"## Adversarial validation\\n\\n```json\\n{\\\"status\\\":\\\"passed\\\",\\\"probes\\\":[{\\\"outcome\\\":\\\"falsified\\\"},{\\\"outcome\\\":\\\"falsified\\\"}]}\\n```\\n\\nHead SHA: `old-head`"}]]' "$head_sha" ;; @@ -541,10 +562,11 @@ assert_missing_coderabbit_waits_for_adversarial_opencode_approval() { assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval() { local temp_dir temp_dir="$(mktemp -d)" - run_gate missing_coderabbit_with_adversarial_approval "$temp_dir" + run_gate missing_coderabbit_with_adversarial_approval_stale "$temp_dir" assert_exit_code 0 "$temp_dir" assert_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" + assert_in_file 'Stale CHANGES_REQUESTED review decision is superseded by current-head robot evidence' "$temp_dir/output.txt" assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" assert_in_file 'conclusion=success' "$temp_dir/gh.log" } @@ -834,11 +856,42 @@ assert_changes_requested_creates_marker_comment() { run_gate changes_requested "$temp_dir" assert_exit_code 0 "$temp_dir" - assert_in_file 'Review decision is CHANGES_REQUESTED' "$temp_dir/gh.log" + assert_in_file 'Review decision remains CHANGES_REQUESTED; await current-head robot review evidence before merge.' "$temp_dir/gh.log" assert_in_file '' "$temp_dir/gh.log" assert_not_in_file '^pr merge' "$temp_dir/gh.log" } +assert_stale_changes_requested_is_superseded_by_current_robot_evidence() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate changes_requested_current_coderabbit "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Stale CHANGES_REQUESTED review decision is superseded by current-head robot evidence' "$temp_dir/output.txt" + assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" + assert_not_in_file 'Review decision is CHANGES_REQUESTED' "$temp_dir/gh.log" +} + +assert_current_changes_requested_remains_blocking() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate changes_requested_current_review "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Review decision is CHANGES_REQUESTED; address current-head requested changes before merge.' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" +} + +assert_current_changes_requested_superseded_by_later_approval() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate changes_requested_current_review_superseded "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Review decision remains CHANGES_REQUESTED; await current-head robot review evidence before merge.' "$temp_dir/gh.log" + assert_not_in_file 'address current-head requested changes before merge' "$temp_dir/gh.log" +} + assert_passing_gate_is_metadata_only_without_merge() { local temp_dir temp_dir="$(mktemp -d)" @@ -956,6 +1009,9 @@ assert_truncated_review_thread_comments_metadata_blocks assert_github_code_quality_current_review_comment_blocks assert_coderabbit_stale_review_comment_does_not_block assert_changes_requested_creates_marker_comment +assert_stale_changes_requested_is_superseded_by_current_robot_evidence +assert_current_changes_requested_remains_blocking +assert_current_changes_requested_superseded_by_later_approval assert_passing_gate_is_metadata_only_without_merge assert_no_required_checks_waits_without_hard_comment assert_self_gate_failure_does_not_recurse