tools: keep multi-commit PRs in the commit queue

.github/workflows/commit-queue.yml

@@ -97,6 +97,11 @@ jobs:
           curl -fsSLo "$readme" "https://github.com/${GITHUB_REPOSITORY}/raw/${GITHUB_SHA}/README.md"
 
           numbers=
+          needs_policy=
+          has_multiple_commits_without_policy() {
+            gh pr view "$1" --json commits,labels --jq \
+              '((.commits | length) > 1) and (([.labels[].name] | index("commit-queue-squash")) == null) and (([.labels[].name] | index("commit-queue-rebase")) == null)'
+          }
           # shellcheck disable=SC2086
           for pr in $CANDIDATES; do
             metadata="${RUNNER_TEMP}/metadata-${pr}.json"
@@ -137,6 +142,11 @@ jobs:
             }
 
             if [ "$metadata_status" -eq 0 ]; then
+              if [ "$(has_multiple_commits_without_policy "$pr")" = "true" ]; then
+                echo "pr ${pr} skipped, multiple commits require commit-queue-squash or commit-queue-rebase"
+                needs_policy="$needs_policy $pr"
+                continue
+              fi
               echo "pr ${pr} is ready for the commit queue"
               numbers="$numbers $pr"
               continue
@@ -154,11 +164,31 @@ jobs:
           done
 
           numbers=$(echo "$numbers" | xargs)
+          needs_policy=$(echo "$needs_policy" | xargs)
           echo "numbers=$numbers" >> "$GITHUB_OUTPUT"
+          echo "needs_policy=$needs_policy" >> "$GITHUB_OUTPUT"
         env:
           CANDIDATES: ${{ needs.get_candidate_prs.outputs.candidates }}
           GH_TOKEN: ${{ github.token }}
 
+      - name: Request multiple-commit policy labels
+        if: steps.get_mergeable_prs.outputs.needs_policy != ''
+        run: |
+          comment='Commit Queue skipped this pull request because it has more than one commit. Add the `commit-queue-squash` label to land it as one commit, or `commit-queue-rebase` to land the commits separately. The `commit-queue` label was left in place.'
+          # shellcheck disable=SC2086
+          for pr in $NEEDS_POLICY; do
+            comments=$(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues/${pr}/comments" \
+              --jq '.[] | select(.user.login=="nodejs-github-bot") | .body')
+            if printf '%s\n' "$comments" | grep -q 'commit-queue-squash' &&
+               printf '%s\n' "$comments" | grep -q 'commit-queue-rebase'; then
+              continue
+            fi
+            gh pr comment "$pr" --body "$comment"
+          done
+        env:
+          GH_TOKEN: ${{ secrets.GH_USER_TOKEN }}
+          NEEDS_POLICY: ${{ steps.get_mergeable_prs.outputs.needs_policy }}
+
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0  # v7.0.0
         if: steps.get_mergeable_prs.outputs.numbers != ''
         with:

doc/contributing/commit-queue.md

@@ -7,9 +7,10 @@ Commit Queue is a feature for the project which simplifies the
 landing process by automating it via GitHub Actions. With it, collaborators can
 queue pull requests for landing by adding the `commit-queue` label to a PR. The
 selector checks readiness with `@node-core/utils`. If the pull request is only
-blocked on a deferrable condition, currently wait time, the queue leaves the
-label in place and retries later. Other failures continue to the existing
-landing and failure-reporting path.
+blocked on a deferrable condition, currently wait time or a missing
+multiple-commit policy label, the queue leaves the label in place and retries
+later. Other failures continue to the existing landing and failure-reporting
+path.
 
 This document gives an overview of how the Commit Queue works, as well as
 implementation details, reasoning for design choices, and current limitations.
@@ -35,14 +36,18 @@ From a high-level, the Commit Queue works as follows:
    2. If the metadata check exits with a deferrable readiness code, meaning
       the PR is only blocked on wait time, keep the `commit-queue` label and
       skip this PR until a later queue run
-   3. Run `git node land` for ready PRs and PRs with hard or mixed readiness
+   3. If a ready PR has more than one commit and neither `commit-queue-squash`
+      nor `commit-queue-rebase` is set, keep the `commit-queue` label, leave a
+      comment asking for one of those labels, and skip this PR until a later
+      queue run
+   4. Run `git node land` for ready PRs and PRs with hard or mixed readiness
       failures, keeping the `commit-queue` label in place during the attempt
-   4. If it fails:
+   5. If it fails:
       1. Replace the `commit-queue` label with the `commit-queue-failed` label
       2. Leave a comment on the PR with the output from `git node land`
       3. Abort the `git node land` session. If the abort succeeds, continue to
          the next PR; otherwise, stop the queue in an unknown state
-   5. If it succeeds:
+   6. If it succeeds:
       1. Push or merge the changes into nodejs/node
       2. Leave a comment on the PR with `Landed in ...`
       3. Close the PR
@@ -54,6 +59,9 @@ first one, add the `commit-queue-squash` label.
 To make the Commit Queue land a pull request containing several commits, add the
 `commit-queue-rebase` label. When using this option, make sure
 that all commits are self-contained, meaning every commit should pass all tests.
+A pull request with more than one commit must have one of those labels. Without
+it, the queue keeps the `commit-queue` label and comments once instead of
+marking the pull request as `commit-queue-failed`.
 
 ## Current limitations
 
@@ -111,8 +119,10 @@ the workflow commit's README without checking out the repository and runs
 fetch, or merge the PR. The filter consumes the structured metadata result
 and its exit code instead of matching human-readable output:
 
-* exit code `0`: the PR is ready and is passed to
-  [`commit-queue.sh`](../../tools/actions/commit-queue.sh)
+* exit code `0`: the PR is ready. If it has more than one commit and neither
+  `commit-queue-squash` nor `commit-queue-rebase` is set, the filter keeps the
+  `commit-queue` label, comments once, and retries later. Otherwise it is
+  passed to [`commit-queue.sh`](../../tools/actions/commit-queue.sh)
 * exit codes `20`-`29`: the PR is not ready for a deferrable metadata reason,
   currently wait time, so it keeps the `commit-queue` label and is retried
   later
@@ -143,19 +153,24 @@ failure path.
    a pull request with commit-queue set.
 
 The script iterates over the pull requests. For each PR, it uses GitHub CLI to
-fetch the labels and select the multiple-commit policy, then runs
-`git node land`, forwarding stdout and stderr to a file. It does not perform a
-separate CI preflight; `git node land` performs the current readiness and CI
-validation.
+fetch the labels and commit count and select the multiple-commit policy. If the
+PR has more than one commit and neither `commit-queue-squash` nor
+`commit-queue-rebase` is set, the script keeps the `commit-queue` label,
+comments once asking for one of those labels, and continues to the next PR.
+Otherwise it runs `git node land`, forwarding stdout and stderr to a file. It
+does not perform a separate CI preflight; `git node land` performs the current
+readiness and CI validation.
 
 The script keeps the `commit-queue` label in place while `git node land` is
-running. PRs that are only blocked on wait time should have already been
-filtered by the metadata check. A hard or mixed readiness failure is passed
-through so `git node land` can produce the failure output. If the landing
-attempt fails for that or any other reason, the job replaces the
-`commit-queue` label with `commit-queue-failed`, leaves a comment with the
-output, and then aborts the landing session. If the abort fails, the queue
-stops instead of continuing in an unknown state.
+running. PRs that are only blocked on wait time or a missing multiple-commit
+policy label should have already been filtered. A hard or mixed readiness
+failure is passed through so `git node land` can produce the failure output.
+If `git node land` still refuses to finish because a multiple-commit policy
+label is missing, the script keeps the `commit-queue` label instead of marking
+the PR as failed. If the landing attempt fails for any other reason, the job
+replaces the `commit-queue` label with `commit-queue-failed`, leaves a comment
+with the output, and then aborts the landing session. If the abort fails, the
+queue stops instead of continuing in an unknown state.
 
 Fast-tracked PRs use the metadata check before checkout and the landing script.
 If the fast-track request has not yet received enough collaborator thumbs-up,

tools/actions/commit-queue.sh

@@ -27,23 +27,50 @@
   rm output
 }
 
+MULTIPLE_COMMIT_POLICY_COMMENT="Commit Queue skipped this pull request because it has more than one commit. Add the \`commit-queue-squash\` label to land it as one commit, or \`commit-queue-rebase\` to land the commits separately. The \`commit-queue\` label was left in place."
+
+already_requested_multiple_commit_policy() {
+  pr=$1
+  comments=$(gh api --paginate "repos/${OWNER}/${REPOSITORY}/issues/${pr}/comments" \
+    --jq '.[] | select(.user.login=="nodejs-github-bot") | .body')
+
+  printf '%s\n' "$comments" | grep -q 'commit-queue-squash' &&
+    printf '%s\n' "$comments" | grep -q 'commit-queue-rebase'
+}
+
+request_multiple_commit_policy() {
+  pr=$1
+
+  echo "pr ${pr} skipped, multiple commits require commit-queue-squash or commit-queue-rebase"
+  if already_requested_multiple_commit_policy "$pr"; then
+    return
+  fi
+  gh pr comment "$pr" --body "${MULTIPLE_COMMIT_POLICY_COMMENT}"
+}
+
 # TODO(mmarchini): should this be set with whoever added the label for each PR?
 git config --local user.email "github-bot@iojs.org"
 git config --local user.name "Node.js GitHub Bot"
 
 SHOULD_ABORT=
 
 for pr in "$@"; do
-  gh pr view "$pr" --json labels --jq ".labels" > labels.json
-  
-  if jq -e 'map(.name) | index("commit-queue-squash")' < labels.json; then
+  gh pr view "$pr" --json labels,commits > pr.json
+
+  if jq -e '.labels | map(.name) | index("commit-queue-squash")' < pr.json; then
     MULTIPLE_COMMIT_POLICY="--fixupAll"
-  elif jq -e 'map(.name) | index("commit-queue-rebase")' < labels.json; then
+  elif jq -e '.labels | map(.name) | index("commit-queue-rebase")' < pr.json; then
     MULTIPLE_COMMIT_POLICY=""
   else
     MULTIPLE_COMMIT_POLICY="--oneCommitMax"
   fi
 
+  if [ "$MULTIPLE_COMMIT_POLICY" = "--oneCommitMax" ] &&
+     [ "$(jq '.commits | length' < pr.json)" -gt 1 ]; then
+    request_multiple_commit_policy "$pr"
+    continue
+  fi
+
   if [ -n "$SHOULD_ABORT" ]; then
     # If `git node land --abort` fails, we're in unknown state. Better to stop
     # the script here, current PR was removed from the queue so it shouldn't
@@ -59,6 +86,13 @@
   # TODO(mmarchini): workaround for ncu not returning the expected status code,
   # if the "Landed in..." message was not on the output we assume land failed
   if ! grep -q '. Post "Landed in .*/pull/'"${pr}" output; then
+    # git node land --oneCommitMax refuses to finish after a successful
+    # autorebase when the PR has multiple commits and no squash/rebase label.
+    if grep -q 'commit-queue-squash' output && grep -q 'commit-queue-rebase' output; then
+      request_multiple_commit_policy "$pr"
+      SHOULD_ABORT=1
+      continue
+    fi
     commit_queue_failed "$pr"
     # Using a variable as there's no point in aborting if there are no PRs left in the queue.
     SHOULD_ABORT=1
@@ -105,4 +139,4 @@
   gh pr edit "$pr" --remove-label "$COMMIT_QUEUE_LABEL" || true
 done
 
-rm -f labels.json
+rm -f pr.json