Skip to content

Commit a18f58e

Browse files
panvapanva
committed
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 <panva.ip@gmail.com>
1 parent 2dea7a3 commit a18f58e

3 files changed

Lines changed: 59 additions & 29 deletions

File tree

‎docs/ncu-ci.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ job after the main PR CI job is started successfully.
177177

178178
`ncu-ci resume <prid>` resumes the latest `node-test-pull-request` CI run linked
179179
in the PR description, comments, or reviews. The job must have finished with
180-
`FAILURE` or `ABORTED` and expose Jenkins' resume action. Running jobs and jobs with
181-
other results are not resumed. If no PR CI run is found, the command reports that
182-
and exits unsuccessfully.
180+
`FAILURE` or `ABORTED`. Running jobs and jobs with other results are not resumed.
181+
If no PR CI run is found, or Jenkins rejects the resume request, the command
182+
reports the failure and exits unsuccessfully.
183183

184184
The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD.
185185
The command refuses to resume if they differ or the approved commit cannot be

‎lib/ci/resume_ci.js‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,13 @@ export class ResumePRJob {
8888
cli.stopSpinner(`Found PR CI job ${job.jobid}`);
8989

9090
const build = new PRBuild(cli, request, job.jobid, undefined,
91-
'result,building,actions[_class,parameters[name,value]]');
91+
'result,building,actions[parameters[name,value]]');
9292
const { result, building, actions = [] } = await build.getBuildData();
9393
if (building || (result !== 'FAILURE' && result !== 'ABORTED')) {
9494
const status = building ? 'RUNNING' : result ?? 'RUNNING';
9595
cli.error(`CI job ${job.jobid} is in status ${status}, skipping resume`);
9696
return false;
9797
}
98-
if (!actions.some(action =>
99-
action._class === 'com.tikal.jenkins.plugins.multijob.MultiJobResumeBuild')) {
100-
cli.error(`CI job ${job.jobid} is not resumable`);
101-
return false;
102-
}
103-
10498
const approvedSHAs = new Set(actions.flatMap(action => action.parameters ?? [])
10599
.filter(parameter => parameter.name === 'COMMIT_SHA_CHECK')
106100
.map(parameter => parameter.value));
@@ -127,7 +121,9 @@ export class ResumePRJob {
127121
}
128122

129123
cli.startSpinner(`Resuming PR CI job ${job.jobid}`);
130-
const response = await request.fetch(`${build.jobUrl}resume`, {
124+
// Jenkins does not export its resume action in the build API. Let the
125+
// resume endpoint determine whether the action is available.
126+
const response = await request.fetch(`${build.jobUrl}resume/`, {
131127
method: 'POST',
132128
headers: {
133129
'Jenkins-Crumb': crumb

‎test/unit/ci_resume.test.js‎

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { tmpdir } from 'node:os';
66
import { join } from 'node:path';
77
import { fileURLToPath } from 'node:url';
88
import * as sinon from 'sinon';
9+
import { fetch, MockAgent } from 'undici';
910

1011
import { ResumePRJob } from '../../lib/ci/resume_ci.js';
1112
import { CI_CRUMB_URL } from '../../lib/ci/run_ci.js';
@@ -14,12 +15,13 @@ import Request from '../../lib/request.js';
1415
import { PRBuild } from '../../lib/ci/build-types/pr_build.js';
1516

1617
const approvedSHA = 'a'.repeat(40);
17-
const resumeTree = 'result,building,actions[_class,parameters[name,value]]';
18+
const resumeTree = 'result,building,actions[parameters[name,value]]';
1819
const resumeBuildData = {
1920
result: 'FAILURE',
2021
building: false,
2122
actions: [
22-
{ _class: 'com.tikal.jenkins.plugins.multijob.MultiJobResumeBuild' },
23+
// Jenkins does not export MultiJobResumeBuild, so the action serializes as {}.
24+
{},
2325
{ parameters: [{ name: 'COMMIT_SHA_CHECK', value: approvedSHA }] }
2426
]
2527
};
@@ -109,9 +111,9 @@ describe('Jenkins resume', () => {
109111
jobRunner = new ResumePRJob(cli, request, owner, repo, prid);
110112
});
111113

112-
it('resumes the PR job with a Jenkins crumb', async() => {
114+
it('resumes the PR job with a Jenkins crumb and an unexported resume action', async() => {
113115
assert.equal(await jobRunner.resume(), true);
114-
sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume`, {
116+
sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, {
115117
method: 'POST',
116118
headers: { 'Jenkins-Crumb': crumb }
117119
});
@@ -122,6 +124,30 @@ describe('Jenkins resume', () => {
122124
assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']);
123125
});
124126

127+
it('posts to the resume handler without redirecting the POST to a GET', async(t) => {
128+
const agent = new MockAgent();
129+
agent.disableNetConnect();
130+
t.after(() => agent.close());
131+
const pool = agent.get('https://ci.nodejs.org');
132+
const jobPath = new URL(jobURL).pathname;
133+
const projectPath = '/job/node-test-pull-request/';
134+
// Stapler redirects a slashless action URL before invoking its POST handler.
135+
pool.intercept({ path: `${jobPath}resume`, method: 'POST' })
136+
.reply(302, '', { headers: { location: `${jobPath}resume/` } });
137+
pool.intercept({ path: `${jobPath}resume/`, method: 'GET' }).reply(405, '');
138+
let resumed = 0;
139+
pool.intercept({ path: `${jobPath}resume/`, method: 'POST' }).reply(() => {
140+
resumed++;
141+
return { statusCode: 302, data: '', responseOptions: { headers: { location: projectPath } } };
142+
});
143+
pool.intercept({ path: projectPath, method: 'GET' }).reply(200, '');
144+
request.fetch.callsFake((url, options) => fetch(url, { ...options, dispatcher: agent }));
145+
146+
assert.equal(await jobRunner.resume(), true);
147+
assert.equal(resumed, 1);
148+
assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']);
149+
});
150+
125151
it('uses the latest PR CI link across the whole thread', async() => {
126152
request.gql.withArgs('PRComments').resolves([
127153
comment('https://ci.nodejs.org/job/node-test-commit/987654/', '2026-09-10T12:00:00Z'),
@@ -130,7 +156,7 @@ describe('Jenkins resume', () => {
130156
]);
131157
request.gql.withArgs('Reviews').resolves([comment(jobURL)]);
132158
assert.equal(await jobRunner.resume(), true);
133-
assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume`);
159+
assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`);
134160
});
135161

136162
it('finds CI links in the PR description', async() => {
@@ -141,7 +167,7 @@ describe('Jenkins resume', () => {
141167
}
142168
});
143169
assert.equal(await jobRunner.resume(), true);
144-
assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume`);
170+
assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`);
145171
});
146172

147173
for (const comments of [[], [comment('https://ci.nodejs.org/job/node-test-commit/123456/')]]) {
@@ -174,7 +200,7 @@ describe('Jenkins resume', () => {
174200
sinon.assert.notCalled(request.fetch);
175201
});
176202

177-
it('resumes an aborted job with a resume action', async() => {
203+
it('resumes an aborted job with an unexported resume action', async() => {
178204
request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result: 'ABORTED' });
179205
request.json.withArgs(fullAPIURL).resolves({ ...failureBuildData, result: 'ABORTED' });
180206
assert.equal(await jobRunner.resume(), true);
@@ -191,11 +217,17 @@ describe('Jenkins resume', () => {
191217
});
192218

193219
for (const result of ['FAILURE', 'ABORTED']) {
194-
it(`refuses a ${result} job without a resume action`, async() => {
195-
request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result, actions: [] });
220+
it(`reports an unavailable resume endpoint for a ${result} job`, async() => {
221+
request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result });
222+
request.fetch.resolves({ status: 404, statusText: 'Not Found' });
196223
assert.equal(await jobRunner.resume(), false);
197-
sinon.assert.notCalled(request.fetch);
198-
assert.deepEqual(cli._calls.error, [[`CI job ${jobid} is not resumable`]]);
224+
sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, {
225+
method: 'POST',
226+
headers: { 'Jenkins-Crumb': crumb }
227+
});
228+
assert.deepEqual(cli._calls.stopSpinner.at(-1), [
229+
'Failed to resume PR CI: 404 Not Found', cli.SPINNER_STATUS.FAILED
230+
]);
199231
});
200232
}
201233

@@ -485,7 +517,7 @@ describe('ncu-ci resume CLI', () => {
485517
const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`;
486518

487519
function run(t, args, hasCI = true, changedFile = 'README.md',
488-
buildData = resumeBuildData, headSHA = approvedSHA) {
520+
buildData = resumeBuildData, headSHA = approvedSHA, resumeResponse = { status: 200 }) {
489521
const dir = mkdtempSync(join(tmpdir(), 'ncu-ci-resume-'));
490522
t.after(() => rmSync(dir, { recursive: true, force: true }));
491523
writeFileSync(join(dir, 'ncurc'), JSON.stringify({ username: 'test', token: 'test' }));
@@ -518,10 +550,10 @@ describe('ncu-ci resume CLI', () => {
518550
yield Buffer.from(${JSON.stringify(failureLog)});
519551
};
520552
Request.prototype.fetch = async (url, options) => {
521-
assert.equal(url, ${JSON.stringify(`${jobURL}resume`)});
553+
assert.equal(url, ${JSON.stringify(`${jobURL}resume/`)});
522554
assert.equal(options.method, 'POST');
523555
assert.equal(options.headers['Jenkins-Crumb'], 'test-crumb');
524-
return { status: 200 };
556+
return ${JSON.stringify(resumeResponse)};
525557
};
526558
process.argv = [process.execPath, ${JSON.stringify(binary)}, ...${JSON.stringify(args)}];
527559
await import(${JSON.stringify(new URL('../../bin/ncu-ci.js', import.meta.url).href)});
@@ -577,18 +609,20 @@ describe('ncu-ci resume CLI', () => {
577609
assert.doesNotMatch(output, /PR CI job successfully resumed/);
578610
});
579611

580-
it('resumes an aborted job when Jenkins exposes the resume action', (t) => {
612+
it('resumes an aborted job with an unexported resume action', (t) => {
581613
const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'],
582614
true, 'README.md', { ...resumeBuildData, result: 'ABORTED' });
583615
assert.equal(status, 0, output);
584616
assert.match(output, /PR CI job successfully resumed/);
585617
});
586618

587-
it('exits 1 when an aborted job is not resumable', (t) => {
619+
it('exits 1 when Jenkins rejects resuming an aborted job', (t) => {
588620
const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'],
589-
true, 'README.md', { ...resumeBuildData, result: 'ABORTED', actions: [] });
621+
true, 'README.md', { ...resumeBuildData, result: 'ABORTED' }, approvedSHA,
622+
{ status: 404, statusText: 'Not Found' });
590623
assert.equal(status, 1, output);
591-
assert.match(output, /is not resumable/);
624+
assert.match(output, /Failed to resume PR CI: 404 Not Found/);
625+
assert.doesNotMatch(output, /PR CI job successfully resumed/);
592626
});
593627

594628
it('exits 1 when the CI-approved commit differs from the PR HEAD', (t) => {

0 commit comments

Comments
 (0)