From e8bb005e4428139d9a24f7bb8335b58fa81cf28d Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Thu, 19 Dec 2024 17:52:07 +0100
Subject: [PATCH] add validation for dont-land labels

---
 .github/workflows/lint-release-proposal.yml   | 14 +++++--
 .../lint-release-proposal-commit-list.mjs     | 39 +++++++++++++++----
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/.github/workflows/lint-release-proposal.yml b/.github/workflows/lint-release-proposal.yml
index 2a46051664b061..221e81627a8de5 100644
--- a/.github/workflows/lint-release-proposal.yml
+++ b/.github/workflows/lint-release-proposal.yml
@@ -61,9 +61,15 @@ jobs:
           gh api \
             -H "Accept: application/vnd.github+json" \
             -H "X-GitHub-Api-Version: 2022-11-28" \
-            --jq '.commits | map({ smallSha: .sha[0:10], splitTitle: .commit.message|split("\n\n")|first|split(":") })' \
-            "/repos/${GITHUB_REPOSITORY}/compare/v${MAJOR}.x...$GITHUB_SHA" --paginate |\
-          jq -r '.[] | "* [[`" + .smallSha + "`](" + env.GITHUB_SERVER_URL + "/" + env.GITHUB_REPOSITORY + "/commit/" + .smallSha + ")] - **" + (.splitTitle|first) + "**:" + (.splitTitle[1:]|join(":"))' |\
-          node tools/actions/lint-release-proposal-commit-list.mjs "$CHANGELOG_PATH" "$GITHUB_SHA"
+            --jq '.commits.[] | { smallSha: .sha[0:10], splitTitle: .commit.message|split("\n\n")|first|split(":") } + (.commit.message|capture("\nPR-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 }}
diff --git a/tools/actions/lint-release-proposal-commit-list.mjs b/tools/actions/lint-release-proposal-commit-list.mjs
index 493f346f054cd6..74ec4b91f60a8b 100755
--- a/tools/actions/lint-release-proposal-commit-list.mjs
+++ b/tools/actions/lint-release-proposal-commit-list.mjs
@@ -1,17 +1,26 @@
 #!/usr/bin/env node
 
+// Takes a stream of JSON objects as inputs, validates the CHANGELOG contains a
+// line corresponding, then outputs the prURL value.
+//
+// $ ./lint-release-proposal-commit-list.mjs "path/to/CHANGELOG.md" "deadbeef00" <<'EOF'
+// {"prURL":"https://github.com/nodejs/node/pull/56131","smallSha":"d48b5224c0","splitTitle":["doc"," fix module.md headings"]}
+// {"prURL":"https://github.com/nodejs/node/pull/56123","smallSha":"f1c2d2f65e","splitTitle":["doc"," update blog release-post link"]}
+// EOF
+
 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
+// 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 startCommitListing = changelog.indexOf('\n### Commits\n');
-const commitList = changelog.slice(startCommitListing, changelog.indexOf('\n\n<a', startCommitListing))
+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.
@@ -19,7 +28,9 @@ const commitList = changelog.slice(startCommitListing, changelog.indexOf('\n\n<a
 
 let expectedNumberOfCommitsLeft = commitList.match(/\n\* \[/g).length;
 for await (const line of stdinLineByLine) {
-  if (line.includes(RELEASE_COMMIT_SHA.slice(0, 10))) {
+  const { smallSha, splitTitle, 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',
@@ -27,10 +38,24 @@ for await (const line of stdinLineByLine) {
     continue;
   }
 
-  // Revert commit have a special treatment.
-  const fixedLine = line.replace(' - **Revert \"', ' - _**Revert**_ "**');
+  const lineStart = commitList.indexOf(`\n* \[[\`${smallSha}\`]`);
+  assert.notStrictEqual(lineStart, -1, `Cannot find ${smallSha} on the list`);
+  const lineEnd = commitList.indexOf('\n', lineStart + 1);
+
+  const expectedCommitTitle = `${`**${splitTitle.shift()}`.replace('**Revert \"', '_**Revert**_ "**')}**:${splitTitle.join(':')}`;
+  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})`);
 
-  assert(commitList.includes('\n' + fixedLine), `Missing "${fixedLine}" in commit list`);
   expectedNumberOfCommitsLeft--;
+  console.log(prURL);
 }
 assert.strictEqual(expectedNumberOfCommitsLeft, 0, 'Release commit is not the last commit in the proposal');