fix: guide CI recovery without bypassing duplicates

Check resume availability before suggesting a recovery action. Keep
same-commit duplicates blocked when neither the latest run nor an eligible
ancestor can resume, and guide users to inspect the run and recover
manually. Suggest resume-ci only when a resume action is available.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex

Author: panva

docs/ncu-ci.md

@@ -150,6 +150,11 @@ Options:
 - `--owner <owner>`: GitHub repository owner, used when `<prid>` is not a GitHub URL
 - `--repo <repo>`: GitHub repository name, used when `<prid>` is not a GitHub URL
 
+With `--check-for-duplicates`, an existing CI run for the same approved commit
+prevents starting another run, including when a failed or aborted build has no
+eligible resume action. Errors checking resume availability also prevent starting
+another run.
+
 Examples:
 
 Run CI for a PR number using repository information from config or flags:
@@ -189,7 +194,7 @@ ancestor that offers the action, and reports which run it will resume. Each
 ancestor must have finished with `FAILURE` or `ABORTED` and match the PR,
 repository, and approved commit. It does not search unrelated older CI runs or
 skip past a mismatched ancestor. If no eligible ancestor is available, the command
-reports the build URL and a command to start a new CI run. Errors checking
+reports the build URL and exits unsuccessfully. Errors checking
 availability stop the command without attempting to resume.
 
 The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD.

lib/ci/ci_utils.js

@@ -1,4 +1,5 @@
 import qs from 'node:querystring';
+import { getPrURL } from '../links.js';
 
 import {
   CI_DOMAIN,
@@ -13,6 +14,18 @@ export const statusType = {
   UNSTABLE: 'UNSTABLE'
 };
 
+export function getCIActionAdvice(pr, action) {
+  const command = `ncu-ci ${action} ${getPrURL(pr)}`;
+  if (action === 'run') {
+    return 'Check the existing CI run in Jenkins and rebase the PR if needed. ' +
+      `To start a new CI run manually: ${command}`;
+  }
+  if (pr.owner === 'nodejs' && pr.repo === 'node') {
+    return `Resume CI by adding the "resume-ci" label to the PR, or run: ${command}`;
+  }
+  return `Resume CI with: ${command}`;
+}
+
 export function getPath(url) {
   return url.replace(`https://${CI_DOMAIN}/`, '').replace('api/json', '');
 }

lib/ci/resume_ci.js

@@ -5,6 +5,7 @@ import { getPrURL } from '../links.js';
 import { debuglog } from '../verbosity.js';
 import { FailureFileScanner } from './failure_file_scanner.js';
 import { findResumableBuild, getApprovedSHA, RESUME_TREE } from './resumable_build.js';
+import { getCIActionAdvice } from './ci_utils.js';
 
 export class ResumePRJob {
   constructor(cli, request, owner, repo, prid) {
@@ -19,7 +20,7 @@ export class ResumePRJob {
     const { cli } = this;
     cli.stopSpinner(
       `Cannot resume PR CI job ${jobid}: Jenkins does not offer a "Resume build" action. ` +
-      `Start a new CI run with: ncu-ci run ${getPrURL(this)}`,
+      getCIActionAdvice(this, 'run'),
       cli.SPINNER_STATUS.FAILED);
     cli.error(build.jobUrl);
   }

lib/ci/run_ci.js

@@ -10,6 +10,12 @@ import PRData from '../pr_data.js';
 import { debuglog } from '../verbosity.js';
 import PRChecker from '../pr_checker.js';
 import { PRBuild } from './build-types/pr_build.js';
+import { getCIActionAdvice } from './ci_utils.js';
+import {
+  findResumableBuild,
+  getApprovedSHA,
+  RESUME_TREE
+} from './resumable_build.js';
 
 export const CI_CRUMB_URL = `https://${CI_DOMAIN}/crumbIssuer/api/json`;
 const CI_PR_NAME = CI_TYPES.get(CI_TYPES_KEYS.PR).jobName;
@@ -94,12 +100,12 @@ export class RunPRJob {
     if (checkForDuplicates) {
       await this.prData.getComments();
       const { jobid, link } = new JobParser(this.prData.comments).parse().get('PR') ?? {};
-      let actions;
+      let buildData;
       try {
-        ({ actions } = jobid
-          ? (await new PRBuild(cli, request, jobid, undefined, 'actions[parameters[name,value]]')
-              .getBuildData())
-          : {});
+        if (jobid) {
+          buildData = await new PRBuild(cli, request, jobid, undefined, RESUME_TREE)
+            .getBuildData();
+        }
       } catch (err) {
         // Jenkins may no longer have the build (e.g. the record was lost) or
         // may answer with an HTML error page instead of JSON. Do not let that
@@ -108,11 +114,35 @@ export class RunPRJob {
           cli.SPINNER_STATUS.WARN);
         cli.warn('Skipping the duplicate CI check');
       }
-      const { parameters } = actions?.find(a => 'parameters' in a) ?? {};
-      if (parameters?.find(c => c.name === 'COMMIT_SHA_CHECK')?.value === certifySafe) {
+      const matchingSHA = buildData?.actions?.some(action =>
+        action.parameters?.some(parameter =>
+          parameter.name === 'COMMIT_SHA_CHECK' && parameter.value === certifySafe));
+      if (matchingSHA) {
         cli.info('Existing CI run found: ' + link);
-        cli.error('Refusing to start a potentially duplicate CI job. Use the ' +
-          '"Resume build" button in the Jenkins UI, or start a new CI manually.');
+        let advice = 'Check the existing CI run in Jenkins before retrying.';
+        if (getApprovedSHA(buildData) === certifySafe && buildData.building === false &&
+            ['FAILURE', 'ABORTED'].includes(buildData.result)) {
+          try {
+            const { build } = await findResumableBuild(cli, request, {
+              jobid,
+              owner: this.owner,
+              repo: this.repo,
+              prid: this.prid
+            }, buildData);
+            advice = getCIActionAdvice(this, build ? 'resume' : 'run');
+          } catch (err) {
+            cli.error(`Could not check whether existing CI run ${link} can be resumed: ` +
+              err.message);
+            cli.error('Refusing to start a potentially duplicate CI job. ' +
+              'Retry after checking Jenkins access.');
+            return false;
+          }
+        } else if (buildData.building === true) {
+          advice = 'The existing CI run is still in progress.';
+        } else if (buildData.result === 'SUCCESS') {
+          advice = 'CI has already succeeded for this commit.';
+        }
+        cli.error(`Refusing to start a potentially duplicate CI job. ${advice}`);
         return false;
       }
     }

test/unit/ci_resume.test.js

@@ -78,7 +78,8 @@ describe('Jenkins resume', () => {
   const jobURL = `https://ci.nodejs.org/job/node-test-pull-request/${jobid}/`;
   const menuURL = `${jobURL}contextMenu`;
   const unavailableMessage = `Cannot resume PR CI job ${jobid}: Jenkins does not offer a ` +
-    '"Resume build" action. Start a new CI run with: ' +
+    '"Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. ' +
+    'To start a new CI run manually: ' +
     `ncu-ci run https://github.com/${owner}/${repo}/pull/${prid}`;
   const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`;
   const fullAPIURL = new PRBuild(null, null, jobid).apiUrl;
@@ -152,10 +153,26 @@ describe('Jenkins resume', () => {
       assert.deepEqual(cli._calls.stopSpinner.at(-1), [
         unavailableMessage, cli.SPINNER_STATUS.FAILED
       ]);
+      assert.doesNotMatch(cli._calls.stopSpinner.at(-1)[0], /request-ci|resume-ci/);
       assert.deepEqual(cli._calls.error, [[jobURL]]);
     });
   }
 
+  it('suggests manual recovery for an unavailable nodejs/node run', async() => {
+    jobRunner = new ResumePRJob(cli, request, 'nodejs', 'node', prid);
+    menuRequest.resolves({ status: 200, json: async() => ({ items: [] }) });
+    assert.equal(await jobRunner.resume(), false);
+    sinon.assert.notCalled(resumeRequest);
+    assert.deepEqual(cli._calls.stopSpinner.at(-1), [
+      `Cannot resume PR CI job ${jobid}: Jenkins does not offer a "Resume build" action. ` +
+        'Check the existing CI run in Jenkins and rebase the PR if needed. ' +
+        'To start a new CI run manually: ' +
+        `ncu-ci run https://github.com/nodejs/node/pull/${prid}`,
+      cli.SPINNER_STATUS.FAILED
+    ]);
+    assert.doesNotMatch(cli._calls.stopSpinner.at(-1)[0], /request-ci|resume-ci/);
+  });
+
   describe('resume ancestry', () => {
     const ancestorId = jobid - 1;
     const resumeCause = (id, overrides = {}) => ({
@@ -1030,16 +1047,20 @@ describe('ncu-ci resume CLI', () => {
       { status: 404, statusText: 'Not Found' });
     assert.equal(status, 1, output);
     assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/);
-    assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/);
+    assert.ok(output.includes('Check the existing CI run in Jenkins and rebase the PR if needed. ' +
+      'To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/123456'));
+    assert.doesNotMatch(output, /request-ci|resume-ci/);
     assert.doesNotMatch(output, /PR CI job successfully resumed/);
   });
 
-  it('exits 1 with a new-run command when Jenkins offers no resume action', (t) => {
+  it('exits 1 with manual recovery guidance when Jenkins offers no resume action', (t) => {
     const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'],
       true, 'README.md', resumeBuildData, approvedSHA, { status: 200 }, { items: [] });
     assert.equal(status, 1, output);
     assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/);
-    assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/);
+    assert.ok(output.includes('Check the existing CI run in Jenkins and rebase the PR if needed. ' +
+      'To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/123456'));
+    assert.doesNotMatch(output, /request-ci|resume-ci/);
     assert.ok(output.includes(jobURL));
     assert.doesNotMatch(output, /Checking failures|Resuming PR CI|PR CI job successfully resumed/);
   });