fix(ci): 재리뷰 요청을 자동으로 걸고 병합으로 반영한 PR 도 빚으로 센다 (#1450)

.github/scripts/review-debt-policy.test.mjs

@@ -3,6 +3,7 @@ import { readFile } from "node:fs/promises";
 import test from "node:test";
 
 const guard = await readFile(new URL("../workflows/review-debt-guard.yml", import.meta.url), "utf8");
+const requestAll = await readFile(new URL("../workflows/review-request-all.yml", import.meta.url), "utf8");
 
 test("team membership is judged by real permission, not author_association", () => {
     // 웹훅 페이로드의 author_association 은 조직 멤버십이 비공개면 MEMBER 대신
@@ -64,3 +65,43 @@ test("changes requested becomes debt only after explicit rerequest and a fix", (
     assert.match(guard, /\(\.parents \| length\) < 2/);
     assert.match(guard, /select\(\.commit\.committer\.date > \\"\$blocked_at\\"\)/);
 });
+
+test("a fix delivered by a merge commit still counts as a response", () => {
+    // 비병합 커밋만 세면 base 를 끌어와 충돌을 풀며 반영한 PR 이 «미반영» 이 된다.
+    // #1316 은 리뷰가 지목한 파일이 실제로 바뀌고 작성자 응답 코멘트도 2건 달린
+    // 채로 빚에서 빠져 있었다. 커밋과 함께 작성자의 응답을 본다 (#1450).
+    assert.match(guard, /issues\/\$pn\/comments/);
+    assert.match(guard, /pulls\/\$pn\/comments/);
+    assert.match(guard, /select\(\.user\.login == \\"\$pr_author\\"\)/);
+    assert.match(guard, /\[ "\$\{fixed:-0\}" -gt 0 \] \|\| \[ "\$responses" -gt 0 \]/);
+});
+
+test("counts are taken per item so paginated pull requests do not break the comparison", () => {
+    // --paginate 는 페이지마다 jq 를 돌린다. 페이지 단위로 length 를 뽑으면 커밋이
+    // 100건을 넘는 PR 에서 숫자가 여러 줄로 나와 -gt 비교가 죽는다.
+    assert.doesNotMatch(guard, /\| length" 2>\/dev\/null/);
+    assert.match(guard, /\| wc -l \| tr -d ' '/);
+});
+
+test("rerequests are automated so silence cannot pass the guard", () => {
+    // 가드는 변경요청을 낸 리뷰어에게 요청이 다시 걸린 경우에만 빚으로 센다. 그
+    // 되살리기를 작성자 손에 맡기면 아무도 걸지 않아 가드가 통째로 무력해진다 —
+    // 8/29 실측에서 반영까지 끝난 7건이 전원 «빚 아님» 이었다 (#1450).
+    assert.match(requestAll, /^\s*types: \[opened, ready_for_review, reopened, synchronize\]/m);
+    assert.match(requestAll, /^\s{2}rerequest:/m);
+    assert.match(requestAll, /github\.event\.action == 'synchronize'/);
+    assert.match(requestAll, /--add-reviewer "\$blocked"/);
+    // 기존 전원 요청은 반영 커밋마다 다시 돌지 않는다.
+    assert.match(requestAll, /github\.event\.action != 'synchronize'/);
+});
+
+test("the rerequest job and the guard judge by the same latest decision", () => {
+    // 한쪽만 «PR 전체의 최신 판정» 을 보면 자동 재요청이 빚으로 이어지지 않거나,
+    // 이미 승인된 PR 에 요청을 되살린다.
+    for (const wf of [guard, requestAll]) {
+        assert.match(wf, /sort_by\(\.t\) \| last/);
+        assert.match(wf, /CHANGES_REQUESTED/);
+    }
+    // 봇·fork 는 토큰이 read-only 라 요청을 걸 수 없다.
+    assert.match(requestAll, /\[ "\$HEAD_REPO" != "\$REPO" \]/);
+});

.github/workflows/review-debt-guard.yml

@@ -79,7 +79,7 @@ jobs:
           # 아니므로 뺀다(review-request-all 도 draft 에는 요청을 걸지 않는다).
           if ! open_prs=$(gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \
                 --jq ".[] | select(.draft == false) | select(.user.login != \"$AUTHOR\")
-                           | \"\(.number)\t\(.created_at[0:10])\t\(.title)\t\([.requested_reviewers[].login | ascii_downcase] | join(\",\"))\""); then
+                           | \"\(.number)\t\(.created_at[0:10])\t\(.user.login)\t\([.requested_reviewers[].login | ascii_downcase] | join(\",\"))\t\(.title)\""); then
             echo "::error::열린 PR 조회에 실패했다 — 근거 없이 PR 을 닫지 않는다. 재실행할 것"
             exit 1
           fi
@@ -102,7 +102,7 @@ jobs:
           # 뺀 실질 커밋으로만 센다.
           debt_file=$(mktemp)
           reviews_file=$(mktemp)
-          while IFS=$'\t' read -r pn created title requested_reviewers; do
+          while IFS=$'\t' read -r pn created pr_author requested_reviewers title; do
             [ -n "$pn" ] || continue
 
             gh api "repos/$REPO/pulls/$pn/reviews" --paginate \
@@ -135,12 +135,34 @@ jobs:
             fi
             IFS=$'\t' read -r blocked_by blocked_at <<< "$blocked"
 
-            fixed=$(gh api "repos/$REPO/pulls/$pn/commits?per_page=100" --paginate \
-              --jq "[.[] | select((.parents | length) < 2)
-                         | select(.commit.committer.date > \"$blocked_at\")] | length" 2>/dev/null || true)
-
-            if [ "${fixed:-0}" -gt 0 ]; then
-              echo "#$pn — @$blocked_by 재리뷰 요청 뒤 변경요청 반영(${fixed}커밋), 아직 미응답"
+            # 반영을 비병합 커밋으로만 세면 병합으로 반영한 PR 을 놓친다. base 를
+            # 끌어와 충돌을 풀며 지적을 반영하면 그 뒤 커밋이 전부 parents 2개라
+            # «미반영» 이 된다 — #1316 은 리뷰가 지목한 파일이 실제로 바뀌고 작성자
+            # 응답 코멘트까지 2건 달린 채로 8/29 까지 빚에서 빠져 있었다(#1450).
+            # 병합 커밋의 first-parent diff 는 base 에서 들어온 변경을 통째로 담아
+            # 반영분과 구별되지 않으므로, 커밋 대신 «작성자가 응답했는가» 를 함께 본다.
+            #
+            # 페이지마다 집계하면 100건이 넘는 PR 에서 숫자가 여러 줄로 나와 비교가
+            # 깨진다. 건별로 뽑아 한 번에 센다.
+            fixed=$( { gh api "repos/$REPO/pulls/$pn/commits?per_page=100" --paginate \
+              --jq ".[] | select((.parents | length) < 2)
+                        | select(.commit.committer.date > \"$blocked_at\") | .sha" 2>/dev/null || true; } \
+              | wc -l | tr -d ' ')
+
+            said=$( { gh api "repos/$REPO/issues/$pn/comments?per_page=100" --paginate \
+              --jq ".[] | select(.user.login == \"$pr_author\")
+                        | select(.created_at > \"$blocked_at\") | .id" 2>/dev/null || true; } \
+              | wc -l | tr -d ' ')
+
+            answered=$( { gh api "repos/$REPO/pulls/$pn/comments?per_page=100" --paginate \
+              --jq ".[] | select(.user.login == \"$pr_author\")
+                        | select(.created_at > \"$blocked_at\") | .id" 2>/dev/null || true; } \
+              | wc -l | tr -d ' ')
+
+            responses=$(( ${said:-0} + ${answered:-0} ))
+
+            if [ "${fixed:-0}" -gt 0 ] || [ "$responses" -gt 0 ]; then
+              echo "#$pn — @$blocked_by 재리뷰 요청 뒤 변경요청 반영(${fixed}커밋 · 응답 ${responses}건), 아직 미응답"
               printf '%s\t%s\t%s\n' "$pn" "$created" "$title" >> "$debt_file"
             else
               echo "#$pn — 변경요청 미반영, 공은 작성자에게"

.github/workflows/review-request-all.yml

@@ -8,6 +8,13 @@
 # "리뷰를 안 하면 새 PR 을 못 연다" 가 성립한다 — 요청을 손으로 걸던 때는
 # 아예 걸지 않는 것으로 가드를 통과할 수 있었다.
 #
+# 같은 구멍이 «변경요청 뒤» 에 그대로 남아 있었다(#1450). GitHub 은 변경요청
+# 리뷰를 제출하는 순간 그 리뷰어의 요청을 떼어 내는데, 가드는 그 리뷰어에게
+# 요청이 다시 걸린 경우에만 빚으로 센다. 되살리는 일이 작성자 손에 맡겨져 있어
+# 8/29 실측에서 반영까지 끝난 PR 7건이 전원 «빚 아님» 이었고, 그중 하나는
+# dependabot 이 작성자라 되살릴 주체 자체가 없었다. 반영 커밋이 올라오면
+# 재요청도 자동으로 건다.
+#
 # 옛 Review Assign(.github/workflows/PRassign.yml)을 대체한다. 그쪽은 develop
 # 에서 파일만 지워졌을 뿐 워크플로 등록이 살아 있어, 파일이 남은 옛 브랜치에서
 # 갈라진 PR 에는 지금도 구버전이 실행된다. 이 워크플로를 들이면서 함께 disable
@@ -16,7 +23,7 @@ name: review-request-all
 
 on:
   pull_request:
-    types: [opened, ready_for_review, reopened]
+    types: [opened, ready_for_review, reopened, synchronize]
 
 permissions:
   pull-requests: write
@@ -25,7 +32,7 @@ jobs:
   request:
     # draft 에는 걸지 않는다. 리뷰할 수 없는 PR 로 남의 미처리 건수를 늘리면
     # review-debt-guard 가 그 사람의 새 PR 을 닫아 버린다.
-    if: github.event.pull_request.draft == false
+    if: github.event.pull_request.draft == false && github.event.action != 'synchronize'
     runs-on: ubuntu-latest
     timeout-minutes: 5
     steps:
@@ -71,3 +78,69 @@ jobs:
 
           echo "리뷰 요청: $targets"
           gh pr edit "$PR_NUMBER" --repo "$REPO" --add-reviewer "$targets"
+
+  # 변경요청 뒤 반영 커밋이 올라오면 그 리뷰어에게 다시 요청을 건다(#1450).
+  #
+  # 판정 기준은 review-debt-guard 와 같아야 한다 — 여기서 거는 요청이 곧 그쪽의
+  # 빚 성립 조건이므로, 한쪽만 «PR 전체의 최신 판정» 을 보면 자동 재요청이 빚으로
+  # 이어지지 않거나 이미 승인된 PR 을 되살린다.
+  rerequest:
+    if: github.event.pull_request.draft == false && github.event.action == 'synchronize'
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: 변경요청 리뷰어에게 재요청
+        env:
+          GH_TOKEN: ${{ github.token }}
+          REPO: ${{ github.repository }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+          AUTHOR: ${{ github.event.pull_request.user.login }}
+          HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
+        run: |
+          set -euo pipefail
+
+          # 봇 PR 은 건너뛴다. dependabot 이 연 PR 에서는 GITHUB_TOKEN 이 read-only 라
+          # 요청을 걸지 못한다 — 시도하면 403 으로 죽을 뿐이다. 봇 PR 의 변경요청이
+          # 빚이 되지 못하는 구멍은 이 워크플로로는 못 메운다(#1450 에 남겨 둔다).
+          case "$AUTHOR" in
+            *'[bot]'|dependabot|github-actions) echo "봇 PR — 건너뜀"; exit 0 ;;
+          esac
+
+          # fork PR 도 토큰이 read-only 다. 팀원 여부를 author_association 으로 가르지
+          # 않는 이유는 review-debt-guard 주석에 있다 — 비공개 멤버가 CONTRIBUTOR 로
+          # 내려와 팀원 대부분이 조용히 빠진다. 실제 제약인 head 저장소로 가른다.
+          if [ "$HEAD_REPO" != "$REPO" ]; then
+            echo "fork PR($HEAD_REPO) — 토큰이 read-only 라 건너뜀"
+            exit 0
+          fi
+
+          # PR 전체의 최신 판정만 본다. 승인 뒤 올라온 커밋은 재요청 대상이 아니다.
+          # --paginate 는 페이지마다 jq 를 돌리므로 정렬은 모아서 한 번에 한다.
+          blocked=$( { gh api "repos/$REPO/pulls/$PR_NUMBER/reviews" --paginate \
+            --jq '.[] | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED")
+                      | {u: .user.login, s: .state, t: .submitted_at}' 2>/dev/null || true; } \
+            | jq -rs 'sort_by(.t) | last // empty | select(.s == "CHANGES_REQUESTED") | .u')
+
+          if [ -z "$blocked" ]; then
+            echo "최신 판정이 변경요청이 아님 — 재요청할 대상 없음"
+            exit 0
+          fi
+
+          # 자기 자신은 리뷰어가 될 수 없다(422). 작성자가 자기 PR 에 변경요청을 낼
+          # 수는 없지만, 소유권이 옮겨진 PR 에서 그렇게 보일 수 있다.
+          if [ "$blocked" = "$AUTHOR" ]; then
+            echo "변경요청을 낸 사람이 작성자 본인 — 건너뜀"
+            exit 0
+          fi
+
+          # 이미 걸려 있으면 다시 걸지 않는다. synchronize 는 force-push 마다 온다.
+          current=$(gh api "repos/$REPO/pulls/$PR_NUMBER" \
+            --jq '[.requested_reviewers[].login] | join(" ")')
+          blocked_lc=$(printf '%s' "$blocked" | tr '[:upper:]' '[:lower:]')
+          current_lc=$(printf '%s' "$current" | tr '[:upper:]' '[:lower:]')
+          case " $current_lc " in
+            *" $blocked_lc "*) echo "@$blocked 요청이 이미 걸려 있음 — 건너뜀"; exit 0 ;;
+          esac
+
+          echo "변경요청 뒤 반영 커밋 — @$blocked 에게 재리뷰 요청"
+          gh pr edit "$PR_NUMBER" --repo "$REPO" --add-reviewer "$blocked"