ci/github-script/prepare: identify real base branch (#435596)

Author: wolfgangwalther

.github/workflows/check.yml

@@ -31,24 +31,7 @@ defaults:
     shell: bash
 
 jobs:
-  no-channel-base:
-    name: no channel base
-    if: contains(fromJSON(inputs.baseBranch).type, 'channel')
-    runs-on: ubuntu-24.04-arm
-    steps:
-      - run: |
-          cat <<EOF
-          The nixos-* and nixpkgs-* branches are pushed to by the channel
-          release script and should not be merged into directly.
-
-          Please target the equivalent release-* branch or master instead.
-          EOF
-          exit 1
-
-  cherry-pick:
-    if: |
-      github.event_name == 'pull_request' ||
-      (fromJSON(inputs.baseBranch).stable && !contains(fromJSON(inputs.headBranch).type, 'development'))
+  commits:
     permissions:
       pull-requests: write
     runs-on: ubuntu-24.04-arm
@@ -68,16 +51,20 @@ jobs:
           GH_TOKEN: ${{ github.token }}
         run: gh api /rate_limit | jq
 
-      - name: Check cherry-picks
+      - name: Check commits
         id: check
         uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
+        env:
+          TARGETS_STABLE: ${{ fromJSON(inputs.baseBranch).stable && !contains(fromJSON(inputs.headBranch).type, 'development') }}
         with:
           script: |
+            const targetsStable = JSON.parse(process.env.TARGETS_STABLE)
             require('./trusted/ci/github-script/commits.js')({
               github,
               context,
               core,
               dry: context.eventName == 'pull_request',
+              cherryPicks: context.eventName == 'pull_request' || targetsStable,
             })
 
       - name: Log current API rate limits

.github/workflows/pr.yml

@@ -23,6 +23,9 @@ permissions: {}
 jobs:
   prepare:
     runs-on: ubuntu-24.04-arm
+    permissions:
+      # wrong branch review comment
+      pull-requests: write
     outputs:
       baseBranch: ${{ steps.prepare.outputs.base }}
       headBranch: ${{ steps.prepare.outputs.head }}
@@ -44,6 +47,7 @@ jobs:
               github,
               context,
               core,
+              dry: context.eventName == 'pull_request',
             })
 
   check:

.github/workflows/push.yml

@@ -2,7 +2,6 @@ name: Push
 
 on:
   push:
-    # Keep this synced with ci/request-reviews/dev-branches.txt
     branches:
       - master
       - staging

ci/github-script/commits.js

@@ -1,7 +1,8 @@
-module.exports = async ({ github, context, core, dry }) => {
+module.exports = async ({ github, context, core, dry, cherryPicks }) => {
   const { execFileSync } = require('node:child_process')
   const { classify } = require('../supportedBranches.js')
   const withRateLimit = require('./withRateLimit.js')
+  const { dismissReviews, postReview } = require('./reviews.js')
 
   await withRateLimit({ github, core }, async (stats) => {
     stats.prs = 1
@@ -16,7 +17,7 @@ module.exports = async ({ github, context, core, dry }) => {
           run_id: context.runId,
           per_page: 100,
         })
-      ).find(({ name }) => name.endsWith('Check / cherry-pick')).html_url +
+      ).find(({ name }) => name.endsWith('Check / commits')).html_url +
         '?pr=' +
         pull_number
 
@@ -137,10 +138,14 @@ module.exports = async ({ github, context, core, dry }) => {
       }
     }
 
-    const commits = await github.paginate(github.rest.pulls.listCommits, {
-      ...context.repo,
-      pull_number,
-    })
+    // For now we short-circuit the list of commits when cherryPicks should not be checked.
+    // This will not run any checks, but still trigger the "dismiss reviews" part below.
+    const commits = !cherryPicks
+      ? []
+      : await github.paginate(github.rest.pulls.listCommits, {
+          ...context.repo,
+          pull_number,
+        })
 
     const extracted = await Promise.all(commits.map(extract))
 
@@ -185,38 +190,10 @@ module.exports = async ({ github, context, core, dry }) => {
 
     // Only create step summary below in case of warnings or errors.
     // Also clean up older reviews, when all checks are good now.
+    // An empty results array will always trigger this condition, which is helpful
+    // to clean up reviews created by the prepare step when on the wrong branch.
     if (results.every(({ severity }) => severity === 'info')) {
-      if (!dry) {
-        await Promise.all(
-          (
-            await github.paginate(github.rest.pulls.listReviews, {
-              ...context.repo,
-              pull_number,
-            })
-          )
-            .filter((review) => review.user.login === 'github-actions[bot]')
-            .map(async (review) => {
-              if (review.state === 'CHANGES_REQUESTED') {
-                await github.rest.pulls.dismissReview({
-                  ...context.repo,
-                  pull_number,
-                  review_id: review.id,
-                  message: 'All cherry-picks are good now, thank you!',
-                })
-              }
-              await github.graphql(
-                `mutation($node_id:ID!) {
-                  minimizeComment(input: {
-                    classifier: RESOLVED,
-                    subjectId: $node_id
-                  })
-                  { clientMutationId }
-                }`,
-                { node_id: review.node_id },
-              )
-            }),
-        )
-      }
+      await dismissReviews({ github, context, dry })
       return
     }
 
@@ -336,45 +313,9 @@ module.exports = async ({ github, context, core, dry }) => {
     const body = core.summary.stringify()
     core.summary.write()
 
-    const pendingReview = (
-      await github.paginate(github.rest.pulls.listReviews, {
-        ...context.repo,
-        pull_number,
-      })
-    ).find(
-      (review) =>
-        review.user.login === 'github-actions[bot]' &&
-        // If a review is still pending, we can just update this instead
-        // of posting a new one.
-        (review.state === 'CHANGES_REQUESTED' ||
-          // No need to post a new review, if an older one with the exact
-          // same content had already been dismissed.
-          review.body === body),
-    )
-
-    if (dry) {
-      if (pendingReview)
-        core.info(`pending review found: ${pendingReview.html_url}`)
-      else core.info('no pending review found')
-    } else {
-      // Either of those two requests could fail for very long comments. This can only happen
-      // with multiple commits all hitting the truncation limit for the diff. If you ever hit
-      // this case, consider just splitting up those commits into multiple PRs.
-      if (pendingReview) {
-        await github.rest.pulls.updateReview({
-          ...context.repo,
-          pull_number,
-          review_id: pendingReview.id,
-          body,
-        })
-      } else {
-        await github.rest.pulls.createReview({
-          ...context.repo,
-          pull_number,
-          event: 'REQUEST_CHANGES',
-          body,
-        })
-      }
-    }
+    // Posting a review could fail for very long comments. This can only happen with
+    // multiple commits all hitting the truncation limit for the diff. If you ever hit
+    // this case, consider just splitting up those commits into multiple PRs.
+    await postReview({ github, context, core, dry, body })
   })
 }