Repository: nodejs/node
Pull request: tools: validate commit list as part of `lint-release-commit`

When updating manually a release proposal, it can be easy to have the commit list in the CHANGELOG and the actual list of commits in the proposal no longer in sync.
It will also double check that none of the commits being released comes from a PR with the `dont-land-onvXX.x`, in case the label was added after the commit was already backported.
I'm also switching to using `make release-only` which contains the checks we were doing and more.

You can test it locally while the v23.5.0 proposal is open:

```sh
#!/bin/bash

set -e
set -o pipefail

GITHUB_REPOSITORY=nodejs/node
MAJOR=22
GITHUB_SHA=7d849ab5e3ff707512c6d7e7031428dafe44a5aa # b0bd12384eca49202d2dd2b4971ee66d7f62f4a0
CHANGELOG_PATH="doc/changelogs/CHANGELOG_V${MAJOR}.md"

curl -L -o "$CHANGELOG_PATH" "https://github.com/nodejs/node/raw/$GITHUB_SHA/$CHANGELOG_PATH"

gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  --jq '.commits.[] | { smallSha: .sha[0:10] } + (.commit.message|capture("^(?<title>.+)\n\n(.*\n)*PR-URL: (?<prURL>.+)\n"))' \
  "/repos/${GITHUB_REPOSITORY}/compare/v${MAJOR}.x...$GITHUB_SHA" --paginate \
| node tools/actions/lint-release-proposal-commit-list.mjs "$CHANGELOG_PATH" "$GITHUB_SHA" \
| while IFS= read -r PR_URL; do
  LABEL="dont-land-on-v${MAJOR}.x" gh pr view \
    --json labels,url \
    --jq 'if (.labels|map(.name==env.LABEL)|any) then error("\(.url) has the \(env.LABEL) label, forbidding it to be in this release proposal") end' \
    "$PR_URL" > /dev/null
done

```


Changed files:
filename: .github/workflows/lint-release-proposal.yml
patch: @@ -33,30 +33,43 @@ jobs:
           echo "COMMIT_SUBJECT=$COMMIT_SUBJECT" >> "$GITHUB_ENV"
       - name: Lint release commit message trailers
         run: |
-          EXPECTED_TRAILER="^PR-URL: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/pull/[[:digit:]]+\$"
+          EXPECTED_TRAILER="^$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/pull/[[:digit:]]+\$"
           echo "Expected trailer format: $EXPECTED_TRAILER"
-          ACTUAL="$(git --no-pager log -1 --format=%b | git interpret-trailers --parse --no-divider)"
+          PR_URL="$(git --no-pager log -1 --format='%(trailers:key=PR-URL,valueonly)')"
           echo "Actual: $ACTUAL"
-          echo "$ACTUAL" | grep -E -q "$EXPECTED_TRAILER"
+          echo "$PR_URL" | grep -E -q "$EXPECTED_TRAILER"
 
-          PR_URL="${ACTUAL:8}"
           PR_HEAD="$(gh pr view "$PR_URL" --json headRefOid -q .headRefOid)"
           echo "Head of $PR_URL: $PR_HEAD"
           echo "Current commit: $GITHUB_SHA"
           [ "$PR_HEAD" = "$GITHUB_SHA" ]
         env:
           GH_TOKEN: ${{ github.token }}
+      - name: Verify it's release-ready
+        run: |
+          SKIP_XZ=1 make release-only
       - name: Validate CHANGELOG
         id: releaser-info
         run: |
           EXPECTED_CHANGELOG_TITLE_INTRO="## $COMMIT_SUBJECT, @"
           echo "Expected CHANGELOG section title: $EXPECTED_CHANGELOG_TITLE_INTRO"
-          CHANGELOG_TITLE="$(grep "$EXPECTED_CHANGELOG_TITLE_INTRO" "doc/changelogs/CHANGELOG_V${COMMIT_SUBJECT:20:2}.md")"
+          MAJOR="$(awk '/^#define NODE_MAJOR_VERSION / { print $3 }' src/node_version.h)"
+          CHANGELOG_PATH="doc/changelogs/CHANGELOG_V${MAJOR}.md"
+          CHANGELOG_TITLE="$(grep "$EXPECTED_CHANGELOG_TITLE_INTRO" "$CHANGELOG_PATH")"
           echo "Actual: $CHANGELOG_TITLE"
           [ "${CHANGELOG_TITLE%%@*}@" = "$EXPECTED_CHANGELOG_TITLE_INTRO" ]
-      - name: Verify NODE_VERSION_IS_RELEASE bit is correctly set
-        run: |
-          grep -q '^#define NODE_VERSION_IS_RELEASE 1$' src/node_version.h
-      - name: Check for placeholders in documentation
-        run: |
-          ! grep "REPLACEME" doc/api/*.md
+          gh api \
+            -H "Accept: application/vnd.github+json" \
+            -H "X-GitHub-Api-Version: 2022-11-28" \
+            --jq '.commits.[] | { smallSha: .sha[0:10] } + (.commit.message|capture("^(?<title>.+)\n\n(.*\n)*PR-URL: (?<prURL>.+)\n"))' \
+            "/repos/${GITHUB_REPOSITORY}/compare/v${MAJOR}.x...$GITHUB_SHA" --paginate \
+          | node tools/actions/lint-release-proposal-commit-list.mjs "$CHANGELOG_PATH" "$GITHUB_SHA" \
+          | while IFS= read -r PR_URL; do
+            LABEL="dont-land-on-v${MAJOR}.x" gh pr view \
+              --json labels,url \
+              --jq 'if (.labels|map(.name==env.LABEL)|any) then error("\(.url) has the \(env.LABEL) label, forbidding it to be in this release proposal") end' \
+              "$PR_URL" > /dev/null
+          done
+        shell: bash  # See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference, we want the pipefail option.
+        env:
+          GH_TOKEN: ${{ github.token }}

filename: tools/actions/lint-release-proposal-commit-list.mjs
patch: @@ -0,0 +1,62 @@
+#!/usr/bin/env node
+
+// Takes a stream of JSON objects as inputs, validates the CHANGELOG contains a
+// line corresponding, then outputs the prURL value.
+//
+// Example:
+// $ git log upstream/vXX.x...upstream/vX.X.X-proposal \
+//     --format='{"prURL":"%(trailers:key=PR-URL,valueonly,separator=)","title":"%s","smallSha":"%h"}' \
+//   | ./lint-release-proposal-commit-list.mjs "path/to/CHANGELOG.md" "$(git rev-parse upstream/vX.X.X-proposal)"
+
+const [,, CHANGELOG_PATH, RELEASE_COMMIT_SHA] = process.argv;
+
+import assert from 'node:assert';
+import { readFile } from 'node:fs/promises';
+import { createInterface } from 'node:readline';
+
+// Creating the iterator early to avoid missing any data:
+const stdinLineByLine = createInterface(process.stdin)[Symbol.asyncIterator]();
+
+const changelog = await readFile(CHANGELOG_PATH, 'utf-8');
+const commitListingStart = changelog.indexOf('\n### Commits\n');
+const commitListingEnd = changelog.indexOf('\n\n<a', commitListingStart);
+const commitList = changelog.slice(commitListingStart, commitListingEnd === -1 ? undefined : commitListingEnd + 1)
+  // Checking for semverness is too expansive, it is left as a exercice for human reviewers.
+  .replaceAll('**(SEMVER-MINOR)** ', '')
+  // Correct Markdown escaping is validated by the linter, getting rid of it here helps.
+  .replaceAll('\\', '');
+
+let expectedNumberOfCommitsLeft = commitList.match(/\n\* \[/g).length;
+for await (const line of stdinLineByLine) {
+  const { smallSha, title, prURL } = JSON.parse(line);
+
+  if (smallSha === RELEASE_COMMIT_SHA.slice(0, 10)) {
+    assert.strictEqual(
+      expectedNumberOfCommitsLeft, 0,
+      'Some commits are listed without being included in the proposal, or are listed more than once',
+    );
+    continue;
+  }
+
+  const lineStart = commitList.indexOf(`\n* [[\`${smallSha}\`]`);
+  assert.notStrictEqual(lineStart, -1, `Cannot find ${smallSha} on the list`);
+  const lineEnd = commitList.indexOf('\n', lineStart + 1);
+
+  const colonIndex = title.indexOf(':');
+  const expectedCommitTitle = `${`**${title.slice(0, colonIndex)}`.replace('**Revert "', '_**Revert**_ "**')}**${title.slice(colonIndex)}`;
+  try {
+    assert(commitList.lastIndexOf(`/${smallSha})] - ${expectedCommitTitle} (`, lineEnd) > lineStart, `Commit title doesn't match`);
+  } catch (e) {
+    if (e?.code === 'ERR_ASSERTION') {
+      e.operator = 'includes';
+      e.expected = expectedCommitTitle;
+      e.actual = commitList.slice(lineStart + 1, lineEnd);
+    }
+    throw e;
+  }
+  assert.strictEqual(commitList.slice(lineEnd - prURL.length - 2, lineEnd), `(${prURL})`, `when checking ${smallSha} ${title}`);
+
+  expectedNumberOfCommitsLeft--;
+  console.log(prURL);
+}
+assert.strictEqual(expectedNumberOfCommitsLeft, 0, 'Release commit is not the last commit in the proposal');