From a18f58e6106a028e8a6c93ebde330b7b1962934e Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Thu, 24 Sep 2026 17:50:33 +0200 Subject: [PATCH] fix: handle unexported Jenkins resume actions Jenkins serializes MultiJobResumeBuild as an empty object in its JSON API. Let the resume endpoint determine availability after the existing checks instead of requiring an exported action class. Assisted-by: Codex Signed-off-by: Filip Skokan --- docs/ncu-ci.md | 6 ++-- lib/ci/resume_ci.js | 12 +++---- test/unit/ci_resume.test.js | 70 +++++++++++++++++++++++++++---------- 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index 57b682cd..8f5d5c21 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -177,9 +177,9 @@ job after the main PR CI job is started successfully. `ncu-ci resume ` resumes the latest `node-test-pull-request` CI run linked in the PR description, comments, or reviews. The job must have finished with -`FAILURE` or `ABORTED` and expose Jenkins' resume action. Running jobs and jobs with -other results are not resumed. If no PR CI run is found, the command reports that -and exits unsuccessfully. +`FAILURE` or `ABORTED`. Running jobs and jobs with other results are not resumed. +If no PR CI run is found, or Jenkins rejects the resume request, the command +reports the failure and exits unsuccessfully. The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD. The command refuses to resume if they differ or the approved commit cannot be diff --git a/lib/ci/resume_ci.js b/lib/ci/resume_ci.js index 0fe679f0..e3bff2e7 100644 --- a/lib/ci/resume_ci.js +++ b/lib/ci/resume_ci.js @@ -88,19 +88,13 @@ export class ResumePRJob { cli.stopSpinner(`Found PR CI job ${job.jobid}`); const build = new PRBuild(cli, request, job.jobid, undefined, - 'result,building,actions[_class,parameters[name,value]]'); + 'result,building,actions[parameters[name,value]]'); const { result, building, actions = [] } = await build.getBuildData(); if (building || (result !== 'FAILURE' && result !== 'ABORTED')) { const status = building ? 'RUNNING' : result ?? 'RUNNING'; cli.error(`CI job ${job.jobid} is in status ${status}, skipping resume`); return false; } - if (!actions.some(action => - action._class === 'com.tikal.jenkins.plugins.multijob.MultiJobResumeBuild')) { - cli.error(`CI job ${job.jobid} is not resumable`); - return false; - } - const approvedSHAs = new Set(actions.flatMap(action => action.parameters ?? []) .filter(parameter => parameter.name === 'COMMIT_SHA_CHECK') .map(parameter => parameter.value)); @@ -127,7 +121,9 @@ export class ResumePRJob { } cli.startSpinner(`Resuming PR CI job ${job.jobid}`); - const response = await request.fetch(`${build.jobUrl}resume`, { + // Jenkins does not export its resume action in the build API. Let the + // resume endpoint determine whether the action is available. + const response = await request.fetch(`${build.jobUrl}resume/`, { method: 'POST', headers: { 'Jenkins-Crumb': crumb diff --git a/test/unit/ci_resume.test.js b/test/unit/ci_resume.test.js index d462c927..084fe3be 100644 --- a/test/unit/ci_resume.test.js +++ b/test/unit/ci_resume.test.js @@ -6,6 +6,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import * as sinon from 'sinon'; +import { fetch, MockAgent } from 'undici'; import { ResumePRJob } from '../../lib/ci/resume_ci.js'; import { CI_CRUMB_URL } from '../../lib/ci/run_ci.js'; @@ -14,12 +15,13 @@ import Request from '../../lib/request.js'; import { PRBuild } from '../../lib/ci/build-types/pr_build.js'; const approvedSHA = 'a'.repeat(40); -const resumeTree = 'result,building,actions[_class,parameters[name,value]]'; +const resumeTree = 'result,building,actions[parameters[name,value]]'; const resumeBuildData = { result: 'FAILURE', building: false, actions: [ - { _class: 'com.tikal.jenkins.plugins.multijob.MultiJobResumeBuild' }, + // Jenkins does not export MultiJobResumeBuild, so the action serializes as {}. + {}, { parameters: [{ name: 'COMMIT_SHA_CHECK', value: approvedSHA }] } ] }; @@ -109,9 +111,9 @@ describe('Jenkins resume', () => { jobRunner = new ResumePRJob(cli, request, owner, repo, prid); }); - it('resumes the PR job with a Jenkins crumb', async() => { + it('resumes the PR job with a Jenkins crumb and an unexported resume action', async() => { assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume`, { + sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, { method: 'POST', headers: { 'Jenkins-Crumb': crumb } }); @@ -122,6 +124,30 @@ describe('Jenkins resume', () => { assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']); }); + it('posts to the resume handler without redirecting the POST to a GET', async(t) => { + const agent = new MockAgent(); + agent.disableNetConnect(); + t.after(() => agent.close()); + const pool = agent.get('https://ci.nodejs.org'); + const jobPath = new URL(jobURL).pathname; + const projectPath = '/job/node-test-pull-request/'; + // Stapler redirects a slashless action URL before invoking its POST handler. + pool.intercept({ path: `${jobPath}resume`, method: 'POST' }) + .reply(302, '', { headers: { location: `${jobPath}resume/` } }); + pool.intercept({ path: `${jobPath}resume/`, method: 'GET' }).reply(405, ''); + let resumed = 0; + pool.intercept({ path: `${jobPath}resume/`, method: 'POST' }).reply(() => { + resumed++; + return { statusCode: 302, data: '', responseOptions: { headers: { location: projectPath } } }; + }); + pool.intercept({ path: projectPath, method: 'GET' }).reply(200, ''); + request.fetch.callsFake((url, options) => fetch(url, { ...options, dispatcher: agent })); + + assert.equal(await jobRunner.resume(), true); + assert.equal(resumed, 1); + assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']); + }); + it('uses the latest PR CI link across the whole thread', async() => { request.gql.withArgs('PRComments').resolves([ comment('https://ci.nodejs.org/job/node-test-commit/987654/', '2026-09-10T12:00:00Z'), @@ -130,7 +156,7 @@ describe('Jenkins resume', () => { ]); request.gql.withArgs('Reviews').resolves([comment(jobURL)]); assert.equal(await jobRunner.resume(), true); - assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume`); + assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`); }); it('finds CI links in the PR description', async() => { @@ -141,7 +167,7 @@ describe('Jenkins resume', () => { } }); assert.equal(await jobRunner.resume(), true); - assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume`); + assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`); }); for (const comments of [[], [comment('https://ci.nodejs.org/job/node-test-commit/123456/')]]) { @@ -174,7 +200,7 @@ describe('Jenkins resume', () => { sinon.assert.notCalled(request.fetch); }); - it('resumes an aborted job with a resume action', async() => { + it('resumes an aborted job with an unexported resume action', async() => { request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result: 'ABORTED' }); request.json.withArgs(fullAPIURL).resolves({ ...failureBuildData, result: 'ABORTED' }); assert.equal(await jobRunner.resume(), true); @@ -191,11 +217,17 @@ describe('Jenkins resume', () => { }); for (const result of ['FAILURE', 'ABORTED']) { - it(`refuses a ${result} job without a resume action`, async() => { - request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result, actions: [] }); + it(`reports an unavailable resume endpoint for a ${result} job`, async() => { + request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result }); + request.fetch.resolves({ status: 404, statusText: 'Not Found' }); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); - assert.deepEqual(cli._calls.error, [[`CI job ${jobid} is not resumable`]]); + sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, { + method: 'POST', + headers: { 'Jenkins-Crumb': crumb } + }); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + 'Failed to resume PR CI: 404 Not Found', cli.SPINNER_STATUS.FAILED + ]); }); } @@ -485,7 +517,7 @@ describe('ncu-ci resume CLI', () => { const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`; function run(t, args, hasCI = true, changedFile = 'README.md', - buildData = resumeBuildData, headSHA = approvedSHA) { + buildData = resumeBuildData, headSHA = approvedSHA, resumeResponse = { status: 200 }) { const dir = mkdtempSync(join(tmpdir(), 'ncu-ci-resume-')); t.after(() => rmSync(dir, { recursive: true, force: true })); writeFileSync(join(dir, 'ncurc'), JSON.stringify({ username: 'test', token: 'test' })); @@ -518,10 +550,10 @@ describe('ncu-ci resume CLI', () => { yield Buffer.from(${JSON.stringify(failureLog)}); }; Request.prototype.fetch = async (url, options) => { - assert.equal(url, ${JSON.stringify(`${jobURL}resume`)}); + assert.equal(url, ${JSON.stringify(`${jobURL}resume/`)}); assert.equal(options.method, 'POST'); assert.equal(options.headers['Jenkins-Crumb'], 'test-crumb'); - return { status: 200 }; + return ${JSON.stringify(resumeResponse)}; }; process.argv = [process.execPath, ${JSON.stringify(binary)}, ...${JSON.stringify(args)}]; await import(${JSON.stringify(new URL('../../bin/ncu-ci.js', import.meta.url).href)}); @@ -577,18 +609,20 @@ describe('ncu-ci resume CLI', () => { assert.doesNotMatch(output, /PR CI job successfully resumed/); }); - it('resumes an aborted job when Jenkins exposes the resume action', (t) => { + it('resumes an aborted job with an unexported resume action', (t) => { const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], true, 'README.md', { ...resumeBuildData, result: 'ABORTED' }); assert.equal(status, 0, output); assert.match(output, /PR CI job successfully resumed/); }); - it('exits 1 when an aborted job is not resumable', (t) => { + it('exits 1 when Jenkins rejects resuming an aborted job', (t) => { const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], - true, 'README.md', { ...resumeBuildData, result: 'ABORTED', actions: [] }); + true, 'README.md', { ...resumeBuildData, result: 'ABORTED' }, approvedSHA, + { status: 404, statusText: 'Not Found' }); assert.equal(status, 1, output); - assert.match(output, /is not resumable/); + assert.match(output, /Failed to resume PR CI: 404 Not Found/); + assert.doesNotMatch(output, /PR CI job successfully resumed/); }); it('exits 1 when the CI-approved commit differs from the PR HEAD', (t) => {