fix: adjust --cleanup to close PRs

Author: RafaelGSS

lib/prepare_security.js

@@ -2,6 +2,7 @@ import fs from 'node:fs';
 import path from 'node:path';
 import auth from './auth.js';
 import Request from './request.js';
+import { parsePRFromURL } from './links.js';
 import {
   NEXT_SECURITY_RELEASE_BRANCH,
   NEXT_SECURITY_RELEASE_FOLDER,
@@ -196,8 +197,7 @@ export default class PrepareSecurityRelease extends SecurityRelease {
     await this.closeAndRequestDisclosure(vulnerabilityJSON.reports);
 
     this.cli.info('Closing pull requests');
-    // For now, close the ones with Security Release label
-    await this.closePRWithLabel('Security Release');
+    await this.closePullRequests(vulnerabilityJSON);
 
     if (vulnerabilityJSON.buildIssue) {
       this.cli.info('Commenting on nodejs/build issue');
@@ -637,26 +637,25 @@ export default class PrepareSecurityRelease extends SecurityRelease {
     this.cli.stopSpinner('Done closing H1 Reports and requesting disclosure');
   }
 
-  async closePRWithLabel(labels) {
-    if (typeof labels === 'string') {
-      labels = [labels];
-    }
-
-    const url = 'https://github.com/nodejs-private/node-private/pull';
+  async closePullRequests(vulnerabilityJSON) {
     this.cli.startSpinner('Closing GitHub Pull Requests...');
-    // At this point, GitHub does not provide filters through their REST API
-    const prs = await this.req.getPullRequest(url);
-    for (const pr of prs) {
-      if (pr.labels.some((l) => labels.includes(l.name))) {
-        this.cli.updateSpinner(`Closing Pull Request: ${pr.number}`);
-        await confirmSecurityStep(
-          this.cli,
-          `close GitHub pull request \`nodejs-private/node-private#${pr.number}\``,
-          'This closes a pull request labeled for the security release.'
-        );
-        await this.req.closePullRequest(pr.number,
-          { owner: 'nodejs-private', repo: 'node-private' });
-      }
+    const entries = [
+      ...vulnerabilityJSON.reports,
+      ...Object.values(vulnerabilityJSON.dependencies ?? {})
+    ];
+    const urls = new Set(entries.flatMap(
+      ({ affectedVersions }) => Object.values(affectedVersions)));
+
+    for (const url of urls) {
+      const { owner, repo, prid } = parsePRFromURL(url);
+      const name = `${owner}/${repo}#${prid}`;
+      this.cli.updateSpinner(`Closing Pull Request: ${name}`);
+      await confirmSecurityStep(
+        this.cli,
+        `close GitHub pull request \`${name}\``,
+        'This closes a pull request listed in vulnerabilities.json.'
+      );
+      await this.req.closePullRequest(prid, { owner, repo });
     }
     this.cli.stopSpinner('Closed GitHub Pull Requests.');
   }

lib/request.js

@@ -158,7 +158,7 @@ export default class Request {
   async closePullRequest(id, { owner, repo }) {
     const url = `/repos/${owner}/${repo}/pulls/${id}`;
     const options = {
-      method: 'POST',
+      method: 'PATCH',
       headers: {
         'Content-Type': 'application/json'
       },

test/unit/request.test.js

@@ -12,6 +12,25 @@ function createRequest(response) {
 }
 
 describe('Request', () => {
+  describe('closePullRequest', () => {
+    it('updates the pull request state with PATCH', async() => {
+      const request = createRequest({});
+      let requestOptions;
+      request.json = async(url, options) => {
+        assert.strictEqual(url, '/repos/nodejs/node/pulls/123');
+        requestOptions = options;
+        return {};
+      };
+
+      await request.closePullRequest(123, { owner: 'nodejs', repo: 'node' });
+
+      assert.strictEqual(requestOptions.method, 'PATCH');
+      assert.deepStrictEqual(JSON.parse(requestOptions.body), {
+        state: 'closed'
+      });
+    });
+  });
+
   describe('query', () => {
     it('preserves detailed GraphQL errors', async() => {
       const variables = { owner: 'nodejs', repo: 'node', prid: 65130 };

test/unit/security_release.test.js

@@ -16,6 +16,55 @@ import {
   getHighestSeverityAnnouncement
 } from '../../lib/security-release/security-release.js';
 
+describe('security_release: cleanup pull requests', () => {
+  const vulnerabilities = {
+    reports: [
+      {
+        affectedVersions: {
+          main: 'https://github.com/nodejs-private/node-private/pull/2',
+          '24.x': 'https://github.com/nodejs-private/node-private/pull/3',
+          '22.x': 'https://github.com/nodejs-private/node-private/pull/3'
+        }
+      }
+    ],
+    dependencies: {
+      undici: {
+        affectedVersions: {
+          main: 'https://github.com/nodejs/node/pull/5'
+        }
+      }
+    }
+  };
+
+  it('closes each PR from affectedVersions once', async() => {
+    const prompts = [];
+    const closed = [];
+    const release = new PrepareSecurityRelease({
+      startSpinner() {},
+      updateSpinner() {},
+      stopSpinner() {},
+      prompt(message) {
+        prompts.push(message);
+        return true;
+      }
+    });
+    release.req = {
+      closePullRequest(prid, repository) {
+        closed.push({ prid, ...repository });
+      }
+    };
+
+    await release.closePullRequests(vulnerabilities);
+
+    assert.deepStrictEqual(closed, [
+      { owner: 'nodejs-private', repo: 'node-private', prid: 2 },
+      { owner: 'nodejs-private', repo: 'node-private', prid: 3 },
+      { owner: 'nodejs', repo: 'node', prid: 5 }
+    ]);
+    assert.strictEqual(prompts.length, closed.length);
+  });
+});
+
 function report(id, rating, affectedVersions = ['24.x']) {
   return {
     id,