chore: sync docs-review-workflows (#25682)

thomasqueirozb · web-flow · commit 5397fe88639a · 2026-06-25T14:31:56.000Z
diff --git a/.github/workflows/add_docs_review_label.yml b/.github/workflows/add_docs_review_label.yml
@@ -1,12 +1,21 @@
-# Adds a label to PRs that touch documentation paths until a maintainer approves.
+# Docs review hold — workflow 1/3
+#
+# Purpose: avoid pulling the docs team into a PR review before the code is
+# directionally approved by a maintainer. This workflow adds
+# "docs review on hold" to any PR touching documentation paths, and the label is
+# cleared automatically once a MEMBER or OWNER approves (see check_docs_review.yml).
+#
+# Design is intentionally best-effort. Edge cases such as an approval being later
+# dismissed without a new commit are accepted — the label corrects itself on the
+# next push. add and check/remove use separate concurrency groups (pr-labels-add-*
+# vs pr-labels-check-*) so a new push cannot cancel a pending approval-check run.
+# Each workflow independently checks state before acting, so concurrent runs
+# across groups are safe.
+#
 # Currently dedicated to the docs review flow. If a second use case appears, promote
 # this into a reusable workflow (`workflow_call`) with the label as an input.
 name: Add Docs Review Label
 
-concurrency:
-  group: pr-labels-${{ github.event.pull_request.number }}
-  cancel-in-progress: false
-
 permissions:
   pull-requests: write
 
@@ -21,6 +30,10 @@ on:
       - "website/content/**"
       - "website/cue/reference/**"
 
+concurrency:
+  group: pr-labels-add-${{ github.event.pull_request.number }}
+  cancel-in-progress: false
+
 env:
   LABEL_NAME: "docs review on hold"
 
@@ -29,34 +42,21 @@ jobs:
     runs-on: ubuntu-24.04
     timeout-minutes: 5
     steps:
-      - name: Check member review state
+      - name: Check member approval
         id: check
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
         with:
           script: |
-            const { data: reviews } = await github.rest.pulls.listReviews({
+            const reviews = await github.paginate(github.rest.pulls.listReviews, {
               owner: context.repo.owner,
               repo: context.repo.repo,
               pull_number: context.payload.pull_request.number,
               per_page: 100,
             });
-
             const allowed = ['MEMBER', 'OWNER'];
-            const latestByUser = new Map();
-            for (const r of reviews) {
-              if (!allowed.includes(r.author_association)) continue;
-              if (r.state === 'COMMENTED') continue;
-              latestByUser.set(r.user.id, r.state);
-            }
-
-            const states = [...latestByUser.values()];
-            const hasApproved = states.includes('APPROVED');
-            const hasChangesRequested = states.includes('CHANGES_REQUESTED');
-            const skip = hasApproved && !hasChangesRequested;
-
-            core.info(`Latest member review states: ${states.join(', ') || '(none)'}`);
-            core.info(`skip=${skip}`);
-            core.setOutput('skip', skip);
+            const approved = reviews.some(r => allowed.includes(r.author_association) && r.state === 'APPROVED');
+            core.info(`Member approval found: ${approved}`);
+            core.setOutput('skip', String(approved));
       - if: steps.check.outputs.skip != 'true'
         uses: actions-ecosystem/action-add-labels@18f1af5e3544586314bbe15c0273249c770b2daf # v1.1.3
         with:
diff --git a/.github/workflows/check_docs_review.yml b/.github/workflows/check_docs_review.yml
@@ -1,40 +1,59 @@
+# Docs review hold — workflow 2/3
+#
+# Fires on every review submission or dismissal. If at least one MEMBER/OWNER
+# has an APPROVED review and the hold label is present, writes the PR number to
+# an artifact for remove_docs_review_label.yml (3/3) to act on.
+#
+# The workflow_run indirection exists for a security reason: pull_request_review
+# events from fork PRs run with a read-only token, so label writes must happen in
+# a separate workflow that runs in the base-repo context with a write token.
+#
+# Review state check is intentionally simple: any APPROVED review from a
+# MEMBER/OWNER is sufficient, regardless of later CHANGES_REQUESTED from the
+# same reviewer. Best-effort is good enough here.
 name: Check Docs Review
 
 on:
   pull_request_review:
-    types: [submitted]
+    types: [submitted, dismissed]
 
 permissions:
   contents: read
   pull-requests: read
 
+concurrency:
+  group: pr-labels-check-${{ github.event.pull_request.number }}
+  cancel-in-progress: false
+
 env:
   LABEL_NAME: "docs review on hold"
 
 jobs:
   check:
-    if: github.event.review.state == 'approved'
     runs-on: ubuntu-24.04
     timeout-minutes: 5
     steps:
-      - name: Check association and label
+      - name: Check approval and label
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
         with:
           script: |
-            const association = context.payload.review.author_association;
-            core.info(`Author association: ${association}`);
-
             const allowed = ['MEMBER', 'OWNER'];
-            if (!allowed.includes(association)) {
-              core.info(`Association "${association}" not in allowed list, skipping`);
-              return;
-            }
+            const reviews = await github.paginate(github.rest.pulls.listReviews, {
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              pull_number: context.payload.pull_request.number,
+              per_page: 100,
+            });
+            const approved = reviews.some(r => allowed.includes(r.author_association) && r.state === 'APPROVED');
+            core.info(`Member approval found: ${approved}`);
+            if (!approved) return;
 
             const labelName = process.env.LABEL_NAME;
-            const { data: labels } = await github.rest.issues.listLabelsOnIssue({
+            const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
               owner: context.repo.owner,
               repo: context.repo.repo,
               issue_number: context.payload.pull_request.number,
+              per_page: 100,
             });
             if (!labels.some(l => l.name === labelName)) {
               core.info(`Label "${labelName}" not found, skipping`);
diff --git a/.github/workflows/remove_docs_review_label.yml b/.github/workflows/remove_docs_review_label.yml
@@ -1,3 +1,10 @@
+# Docs review hold — workflow 3/3
+#
+# Triggered by check_docs_review.yml (2/3) completing successfully. Runs in the
+# base-repo context so it has a write token, which is required to modify labels
+# on fork PRs. Re-checks review state immediately before removal to reduce (but
+# not eliminate) the chance of removing the label while a CHANGES_REQUESTED was
+# submitted in the small window between check and removal. Best-effort is fine.
 name: Remove Docs Review Label
 
 on:
@@ -13,14 +20,14 @@ env:
   LABEL_NAME: "docs review on hold"
 
 jobs:
-  remove_label:
+  download:
     if: github.event.workflow_run.conclusion == 'success'
     runs-on: ubuntu-24.04
     timeout-minutes: 5
     permissions:
       actions: read
-      issues: write
-      pull-requests: write
+    outputs:
+      pr_number: ${{ steps.read.outputs.pr_number }}
     steps:
       - name: Download PR number
         id: download
@@ -31,23 +38,64 @@ jobs:
           run-id: ${{ github.event.workflow_run.id }}
           github-token: ${{ github.token }}
 
-      - name: Remove label
+      - name: Read PR number
+        id: read
         if: steps.download.outcome == 'success'
+        run: |
+          pr_number=$(cat pr-number | tr -d '[:space:]')
+          echo "pr_number=${pr_number}" >> $GITHUB_OUTPUT
+
+  remove_label:
+    needs: download
+    if: needs.download.outputs.pr_number != ''
+    runs-on: ubuntu-24.04
+    timeout-minutes: 5
+    concurrency:
+      group: pr-labels-${{ needs.download.outputs.pr_number }}
+      cancel-in-progress: false
+    permissions:
+      issues: write
+      pull-requests: write
+    steps:
+      - name: Remove label
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
+        env:
+          PR_NUMBER: ${{ needs.download.outputs.pr_number }}
         with:
           script: |
-            const fs = require('fs');
-            const prNumber = parseInt(fs.readFileSync('pr-number', 'utf8').trim(), 10);
+            const prNumber = parseInt(process.env.PR_NUMBER, 10);
             if (isNaN(prNumber)) {
               core.setFailed('Invalid PR number');
               return;
             }
 
-            const labelName = process.env.LABEL_NAME;
-            await github.rest.issues.removeLabel({
+            const allowed = ['MEMBER', 'OWNER'];
+            const reviews = await github.paginate(github.rest.pulls.listReviews, {
               owner: context.repo.owner,
               repo: context.repo.repo,
-              issue_number: prNumber,
-              name: labelName,
+              pull_number: prNumber,
+              per_page: 100,
             });
-            core.info(`Removed "${labelName}" label from PR #${prNumber}`);
+            const approved = reviews.some(r => allowed.includes(r.author_association) && r.state === 'APPROVED');
+            core.info(`Member approval found: ${approved}`);
+            if (!approved) {
+              core.info('No member approval, skipping removal');
+              return;
+            }
+
+            const labelName = process.env.LABEL_NAME;
+            try {
+              await github.rest.issues.removeLabel({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: prNumber,
+                name: labelName,
+              });
+              core.info(`Removed "${labelName}" label from PR #${prNumber}`);
+            } catch (err) {
+              if (err.status === 404) {
+                core.info(`Label "${labelName}" already absent from PR #${prNumber}, skipping`);
+              } else {
+                throw err;
+              }
+            }
