tools: keep not-ready PRs in commit queue

.github/workflows/commit-queue.yml

@@ -33,24 +33,25 @@ jobs:
       - name: Get Pull Requests
         id: get_mergeable_prs
         run: |
-          prs=$(gh pr list \
+          list_prs() {
+            gh pr list \
                   --repo "$GITHUB_REPOSITORY" \
                   --base "$GITHUB_REF_NAME" \
                   --label 'commit-queue' \
+                  "$@" \
                   --json 'number' \
-                  --search "created:<=$(date --date="2 days ago"  +"%Y-%m-%dT%H:%M:%S%z") -label:blocked" \
                   -t '{{ range . }}{{ .number }} {{ end }}' \
-                  --limit 100)
-          fast_track_prs=$(gh pr list \
-                  --repo "$GITHUB_REPOSITORY" \
-                  --base "$GITHUB_REF_NAME" \
-                  --label 'commit-queue' \
+                  --limit 100
+          }
+          ready_prs=$(list_prs \
+                  --search "created:<=$(date --date="2 days ago"  +"%Y-%m-%dT%H:%M:%S%z") -label:blocked")
+          fast_track_prs=$(list_prs \
                   --label 'fast-track' \
-                  --search "-label:blocked" \
-                  --json 'number' \
-                  -t '{{ range . }}{{ .number }} {{ end }}' \
-                  --limit 100)
-          numbers=$(echo $prs' '$fast_track_prs | jq -r -s 'unique | join(" ")')
+                  --search "-label:blocked")
+          queued_prs=$(list_prs \
+                  --search "-label:blocked")
+          numbers=$(printf '%s %s %s\n' "$ready_prs" "$fast_track_prs" "$queued_prs" |
+                  jq -r -s 'reduce .[] as $pr ([]; if index($pr) then . else . + [$pr] end) | join(" ")')
           echo "numbers=$numbers" >> "$GITHUB_OUTPUT"
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

doc/contributing/commit-queue.md

@@ -1,12 +1,14 @@
 # Commit queue
 
-_tl;dr: You can land pull requests by adding the `commit-queue` label to it._
+_tl;dr: You can ask the queue to land pull requests by adding the
+`commit-queue` label to them._
 
 Commit Queue is a feature for the project which simplifies the
 landing process by automating it via GitHub Actions. With it, collaborators can
-land pull requests by adding the `commit-queue` label to a PR. All
-checks will run via `@node-core/utils`, and if the pull request is ready to
-land, the Action will rebase it and push to `main`.
+queue pull requests for landing by adding the `commit-queue` label to a PR. All
+checks will run via `@node-core/utils`. If the pull request is not ready to land
+yet, the queue will leave the label in place and retry later. If it is ready,
+the Action will rebase it and push to `main`.
 
 This document gives an overview of how the Commit Queue works, as well as
 implementation details, reasoning for design choices, and current limitations.
@@ -15,25 +17,34 @@ implementation details, reasoning for design choices, and current limitations.
 
 From a high-level, the Commit Queue works as follow:
 
-1. Collaborators will add `commit-queue` label to pull requests ready to land
-2. Every five minutes the queue will do the following for each mergeable pull request
-   with the label:
+1. Collaborators will add `commit-queue` label to pull requests they want the
+   queue to land. The label can be added before the pull request has completed
+   its wait time or approvals, or before requested CI has finished. The commit
+   queue does not request CI on its own.
+2. On each scheduled run, the queue will do the following for each open pull
+   request with the label. The workflow uses a five-minute cron, but GitHub
+   Actions scheduled workflows are not guaranteed to run exactly every five
+   minutes:
    1. Check if the PR also has a `request-ci` label (if it has, skip this PR
       since it's pending a CI run)
    2. Check if the last Jenkins CI is finished running (if it is not, skip this
       PR)
-   3. Remove the `commit-queue` label
-   4. Run `git node land <pr> --oneCommitMax`
-   5. If it fails:
-      1. Abort `git node land` session
-      2. Add `commit-queue-failed` label to the PR
-      3. Leave a comment on the PR with the output from `git node land`
-      4. Skip next steps, go to next PR in the queue
+   3. Run `git node land <pr> --oneCommitMax`
+   4. If `git node land` reports that the PR is not ready yet because it is
+      waiting for approvals, wait time, or CI, and no hard failure was reported,
+      keep the `commit-queue` label and skip this PR until a later queue run
+   5. If it fails for another reason:
+      1. Remove the `commit-queue` label
+      2. Abort `git node land` session
+      3. Add `commit-queue-failed` label to the PR
+      4. Leave a comment on the PR with the output from `git node land`
+      5. Skip next steps, go to next PR in the queue
    6. If it succeeds:
-      1. Push the changes to nodejs/node
-      2. Leave a comment on the PR with `Landed in ...`
-      3. Close the PR
-      4. Go to next PR in the queue
+      1. Remove the `commit-queue` label
+      2. Push the changes to nodejs/node
+      3. Leave a comment on the PR with `Landed in ...`
+      4. Close the PR
+      5. Go to next PR in the queue
 
 To make the Commit Queue squash all the commits of a pull request into the
 first one, add the `commit-queue-squash` label.
@@ -59,10 +70,15 @@ of the commit queue:
 
 ## Implementation
 
-The [action](../../.github/workflows/commit-queue.yml) will run on scheduler
-events every five minutes. Five minutes is the smallest number accepted by
-the scheduler. The scheduler is not guaranteed to run every five minutes, it
-might take longer between runs.
+The [action](../../.github/workflows/commit-queue.yml) runs on scheduled events.
+It uses a five-minute cron because that is the smallest interval accepted by
+GitHub Actions. Scheduled workflows are not guaranteed to run exactly at that
+cadence and might take longer between runs.
+
+The workflow also uses a concurrency group so only one commit queue run can be
+active at a time. If a scheduled run starts while a previous run is still
+running, GitHub Actions keeps at most one pending run for the same concurrency
+group. A newer pending run replaces an older pending run.
 
 Using the scheduler is preferable over using pull\_request\_target for two
 reasons:
@@ -79,9 +95,12 @@ reasons:
 `@node-core/utils` is configured with a personal token and
 a Jenkins token from
 [@nodejs-github-bot](https://github.com/nodejs/github-bot).
-`octokit/graphql-action` is used to fetch all pull requests with the
-`commit-queue` label. The output is a JSON payload, so `jq` is used to turn
-that into a list of PR ids we can pass as arguments to
+The workflow uses GitHub CLI to fetch pull requests with the `commit-queue`
+label. It first fetches the same age-based and fast-track buckets the queue used
+before accepting early queue requests, then fetches the broader queue and
+de-duplicates the result. This keeps not-yet-ready PRs from crowding out PRs
+that the previous query would have selected if GitHub paginates or caps a query
+result. The output is turned into a list of PR ids we can pass as arguments to
 [`commit-queue.sh`](../../tools/actions/commit-queue.sh).
 
 > The personal token only needs permission for public repositories and to read
@@ -92,8 +111,7 @@ that into a list of PR ids we can pass as arguments to
 
 1. The repository owner
 2. The repository name
-3. The Action GITHUB\_TOKEN
-4. Every positional argument starting at this one will be a pull request ID of
+3. Every positional argument starting at this one will be a pull request ID of
    a pull request with commit-queue set.
 
 The script will iterate over the pull requests. `ncu-ci` is used to check if
@@ -102,15 +120,27 @@ the PR is waiting for CI to start (`request-ci` label). The PR is skipped if CI
 is pending. No other CI validation is done here since `git node land` will fail
 if the last CI failed.
 
-The script removes the `commit-queue` label. It then runs `git node land`,
-forwarding stdout and stderr to a file. If any errors happen,
-`git node land --abort` is run, and then a `commit-queue-failed` label is added
-to the PR, as well as a comment with the output of `git node land`.
-
-If no errors happen during `git node land`, the script will use the
-`GITHUB_TOKEN` to push the changes to `main`, and then will leave a
-`Landed in ...` comment in the PR, and then will close it. Iteration continues
-until all PRs have done the steps above.
+The script runs `git node land`, forwarding stdout and stderr to a file. If the
+output says the PR is not ready yet due to missing approvals, wait time, missing
+or pending CI, or stale review/CI state, and there is no hard failure in the
+same output, the script leaves the `commit-queue` label in place and retries it
+during a later scheduled run. Missing CI still needs to be requested separately.
+If any other errors happen, the script removes the `commit-queue` label, runs
+`git node land --abort`, and then adds a `commit-queue-failed` label to the PR,
+as well as a comment with the output of `git node land`.
+
+Fast-tracked PRs use the same `git node land` checks. If the fast-track request
+has not yet received enough collaborator thumbs-up, the queue keeps the
+`commit-queue` label and retries until either the fast-track request is approved
+or the PR becomes landable through the regular wait-time rules. The commit queue
+does not create the fast-track request comment; that is handled when the
+`fast-track` label is added. If that comment is missing, the queue reports the
+failure instead of keeping the PR queued.
+
+If no errors happen during `git node land`, the script removes the
+`commit-queue` label, uses the `GITHUB_TOKEN` to push the changes to `main`, and
+then will leave a `Landed in ...` comment in the PR, and then will close it.
+Iteration continues until all PRs have done the steps above.
 
 ## Reverting broken commits
 

tools/actions/commit-queue.sh

@@ -27,6 +27,46 @@ commit_queue_failed() {
   rm output
 }
 
+commit_queue_classify_output() {
+  COMMIT_QUEUE_DEFERRED=0
+
+  if grep -Eq \
+    -e 'This PR has conflicts that must be resolved' \
+    -e 'This PR was merged on ' \
+    -e 'This PR was closed on ' \
+    -e 'PR author is a new contributor:' \
+    -e 'No commits detected' \
+    -e '[0-9]+ failure\(s\) on the last Jenkins CI run' \
+    -e '[0-9]+ GitHub CI job\(s\) failed:' \
+    -e '[0-9]+ GitHub CI job\(s\) cancelled:' \
+    -e 'GitHub CI failed with status:' \
+    -e 'Mismatched commits:' \
+    -e 'Failed to apply patches' \
+    -e 'Patch still contains lint errors' \
+    output; then
+    return
+  fi
+
+  # These are @node-core/utils policy/readiness failures that can become
+  # landable later without changing the queue request.
+  if grep -Eq \
+    -e 'Approvals: 0' \
+    -e 'No approving reviews found' \
+    -e 'Requested Changes: [0-9]+' \
+    -e 'Commits were pushed since the last approving review:' \
+    -e 'This PR needs to wait .* to land' \
+    -e 'semver-major requires at least 2 TSC approvals' \
+    -e 'No Jenkins CI runs detected' \
+    -e 'No full Jenkins CI runs detected' \
+    -e 'Commits were pushed after the last .* CI run:' \
+    -e 'Last Jenkins CI still running' \
+    -e 'No GitHub CI runs detected' \
+    -e 'GitHub CI is still running' \
+    output; then
+    COMMIT_QUEUE_DEFERRED=1
+  fi
+}
+
 # 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"
@@ -45,9 +85,6 @@ for pr in "$@"; do
     continue
   fi
 
-  # Delete the commit queue label
-  gh pr edit "$pr" --remove-label "$COMMIT_QUEUE_LABEL"
-
   if jq -e 'map(.name) | index("commit-queue-squash")' < labels.json; then
     MULTIPLE_COMMIT_POLICY="--fixupAll"
   elif jq -e 'map(.name) | index("commit-queue-rebase")' < labels.json; then
@@ -63,6 +100,13 @@ for pr in "$@"; do
   # 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
+    commit_queue_classify_output
+    if [ "$COMMIT_QUEUE_DEFERRED" -eq 1 ]; then
+      echo "pr ${pr} skipped, not ready to land"
+      rm output
+      continue
+    fi
+    gh pr edit "$pr" --remove-label "$COMMIT_QUEUE_LABEL"
     commit_queue_failed "$pr"
     # 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
@@ -71,6 +115,8 @@ for pr in "$@"; do
     continue
   fi
 
+  gh pr edit "$pr" --remove-label "$COMMIT_QUEUE_LABEL"
+
   if [ -z "$MULTIPLE_COMMIT_POLICY" ]; then
     start_sha=$(git rev-parse $UPSTREAM/$DEFAULT_BRANCH)
     end_sha=$(git rev-parse HEAD)