Skip to content

Commit b178d8f

Browse files
panvaMoLow
andcommitted
feat: add ncu-ci resume command
Resume failed or aborted PR CI jobs when Jenkins exposes a resume action and the CI-approved commit still matches the PR head. Refuse when available failure diagnostics reference files changed by the PR. Refs: #642 Co-authored-by: Moshe Atlow <moshe@atlow.co.il> Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex
1 parent 92bf51c commit b178d8f

6 files changed

Lines changed: 837 additions & 7 deletions

File tree

‎bin/ncu-ci.js‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
import {
2424
RunPRJob
2525
} from '../lib/ci/run_ci.js';
26+
import { ResumePRJob } from '../lib/ci/resume_ci.js';
2627
import { writeJson, writeFile } from '../lib/file.js';
2728
import { getMergedConfig } from '../lib/config.js';
2829
import { runPromise } from '../lib/run.js';
@@ -134,6 +135,26 @@ const args = yargs(hideBin(process.argv))
134135
},
135136
handler
136137
})
138+
.command({
139+
command: 'resume <prid>',
140+
desc: 'Resume the latest CI run for given PR',
141+
builder: (yargs) => {
142+
yargs
143+
.positional('prid', {
144+
describe: 'ID of the PR or URL to the PR',
145+
type: 'string'
146+
})
147+
.option('owner', {
148+
default: '',
149+
describe: 'GitHub repository owner'
150+
})
151+
.option('repo', {
152+
default: '',
153+
describe: 'GitHub repository name'
154+
});
155+
},
156+
handler
157+
})
137158
.command({
138159
command: 'url <url>',
139160
desc: 'Automatically detect CI type and show results',
@@ -278,11 +299,13 @@ class RunPRJobCommand {
278299
return this.argv.prid;
279300
}
280301

281-
async start() {
282-
const {
283-
cli, request, prid, repo, owner
284-
} = this;
302+
validate() {
303+
const { cli, prid, repo, owner } = this;
285304
let validArgs = true;
305+
if (!Number.isSafeInteger(prid) || prid <= 0) {
306+
validArgs = false;
307+
cli.error('Pull request ID must be a positive integer');
308+
}
286309
if (!repo) {
287310
validArgs = false;
288311
cli.error('GitHub repository is missing, please set it via ncu-config ' +
@@ -295,6 +318,13 @@ class RunPRJobCommand {
295318
}
296319
if (!validArgs) {
297320
this.cli.setExitCode(1);
321+
}
322+
return validArgs;
323+
}
324+
325+
async start() {
326+
const { cli, request, prid, repo, owner } = this;
327+
if (!this.validate()) {
298328
return;
299329
}
300330
const { certifySafe, checkForDuplicates } = this.argv;
@@ -308,6 +338,20 @@ class RunPRJobCommand {
308338
}
309339
}
310340

341+
class ResumePRJobCommand extends RunPRJobCommand {
342+
async start() {
343+
const { cli, request, prid, repo, owner } = this;
344+
if (!this.validate()) {
345+
return;
346+
}
347+
const jobRunner = new ResumePRJob(cli, request, owner, repo, prid);
348+
if (!(await jobRunner.resume())) {
349+
cli.setExitCode(1);
350+
process.exitCode = 1;
351+
}
352+
}
353+
}
354+
311355
class CICommand {
312356
constructor(cli, request, argv) {
313357
this.cli = cli;
@@ -565,7 +609,8 @@ async function main(command, argv) {
565609
let commandHandler;
566610
// Prepare queue.
567611
switch (command) {
568-
case 'run': {
612+
case 'run':
613+
case 'resume': {
569614
const maybeURL = URL.parse(argv.prid);
570615
if (maybeURL?.host === 'github.com') {
571616
const [, owner, repo, , prid, , commit_sha] = maybeURL.pathname.split('/');
@@ -575,7 +620,8 @@ async function main(command, argv) {
575620
argv.prid = prid;
576621
}
577622
argv.prid = Number(argv.prid);
578-
const jobRunner = new RunPRJobCommand(cli, request, argv);
623+
const Command = command === 'run' ? RunPRJobCommand : ResumePRJobCommand;
624+
const jobRunner = new Command(cli, request, argv);
579625
return jobRunner.start();
580626
}
581627
case 'rate': {

‎docs/ncu-ci.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Commands:
1717
runs
1818
ncu-ci walk <type> Walk the CI and display the failures
1919
ncu-ci run <prid> Start a node-test-pull-request CI job for a PR
20+
ncu-ci resume <prid> Resume the latest node-test-pull-request CI job for a PR
2021
ncu-ci url <url> Automatically detect CI type and show results
2122
ncu-ci pr <jobid> Show results of a node-test-pull-request CI job
2223
ncu-ci commit <jobid> Show results of a node-test-commit CI job
@@ -172,6 +173,36 @@ ncu-ci run https://github.com/nodejs/node/pull/34127/commits/35ea6ded7315cf9d058
172173
If the PR has the `v8 engine` label, `ncu-ci run` also triggers the `node-test-commit-v8-linux`
173174
job after the main PR CI job is started successfully.
174175

176+
### `ncu-ci resume <prid>`
177+
178+
`ncu-ci resume <prid>` resumes the latest `node-test-pull-request` CI run linked
179+
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.
183+
184+
The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD.
185+
The command refuses to resume if they differ or the approved commit cannot be
186+
determined.
187+
188+
Before resuming, the command compares the parsed failures with the PR's changed
189+
files, including previous filenames for renames. It refuses to resume if a failed
190+
test or a file referenced in a failure diagnostic is changed by the PR. Unknown
191+
or unavailable failure details do not prevent resuming; the check uses the
192+
available diagnostics. Failure to retrieve the PR's changed-file list prevents
193+
resuming.
194+
195+
Pass a PR number with repository information from config or flags, or a PR URL:
196+
197+
```sh
198+
ncu-ci resume 34127 --owner nodejs --repo node
199+
ncu-ci resume https://github.com/nodejs/node/pull/34127
200+
```
201+
202+
This uses Jenkins' **Resume build** action on the existing job. It does not start
203+
a fresh CI run for the current PR head. Jenkins credentials with permission to
204+
resume the job are required.
205+
175206
### `ncu-ci pr <jobid>`
176207

177208
`ncu-ci pr <jobid>` returns information about the results of a `node-test-pull-request` job.

‎lib/ci/resume_ci.js‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { JobParser, CI_TYPES_KEYS } from './ci_type_parser.js';
2+
import { CI_CRUMB_URL } from './run_ci.js';
3+
import { PRBuild } from './build-types/pr_build.js';
4+
import { getPrURL } from '../links.js';
5+
import { debuglog } from '../verbosity.js';
6+
7+
function referencesFile(failure, filename) {
8+
const paths = new Set([filename]);
9+
if (filename.startsWith('test/')) {
10+
paths.add(filename.slice(5));
11+
// Node's test runner omits the test directory and file extension in TAP names.
12+
paths.add(filename.slice(5).replace(/\.(?:js|mjs|cjs|out)$/, ''));
13+
} else if (filename.startsWith('lib/')) {
14+
paths.add(filename.slice(4));
15+
}
16+
// Diagnostics can contain both literal Windows paths and escaped string output.
17+
const text = `${failure.file ?? ''}\n${failure.reason}`.replace(/\\+/g, '/');
18+
return [...paths].some(path => {
19+
const escaped = path.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
20+
return new RegExp(`(?:^|[^\\w.-])${escaped}(?=$|[^\\w./-])`, 'm').test(text);
21+
});
22+
}
23+
24+
export class ResumePRJob {
25+
constructor(cli, request, owner, repo, prid) {
26+
this.cli = cli;
27+
this.request = request;
28+
this.owner = owner;
29+
this.repo = repo;
30+
this.prid = prid;
31+
}
32+
33+
async checkFailures(jobid) {
34+
const { cli, request, owner, repo, prid } = this;
35+
const diagnostics = [];
36+
const buildRequest = Object.create(request);
37+
buildRequest.json = async(...args) => {
38+
const data = await request.json(...args);
39+
// Inspect failed descendants even when their parent job was aborted.
40+
return data?.result === 'ABORTED' ? { ...data, result: 'FAILURE' } : data;
41+
};
42+
buildRequest.text = async(...args) => {
43+
const text = await request.text(...args);
44+
// The failure parser can trim the source location before a compiler error.
45+
// Keep complete diagnostic lines from failed builds for the file check.
46+
diagnostics.push(...text.match(/^.*(?:error:|error C\d+:).*$/mg) ?? []);
47+
// Infrastructure failures can take precedence over test failures in the parser.
48+
const tests = text.match(/^not ok \d+[\s\S]+? {2}\.\.\.\r?\n/mg) ?? [];
49+
diagnostics.push(...tests.filter(test => !test.includes('# TODO :')));
50+
return text;
51+
};
52+
const build = new PRBuild(cli, buildRequest, jobid, Infinity);
53+
try {
54+
await build.getResults();
55+
} catch (err) {
56+
debuglog(err);
57+
}
58+
const failures = build.failures.filter(Boolean);
59+
failures.push({ reason: diagnostics.join('\n') });
60+
61+
cli.startSpinner('Checking failures against changed PR files');
62+
const overlaps = new Set();
63+
for await (const file of request.getPullRequestFiles({ owner, repo, prid })) {
64+
for (const filename of [file.filename, file.previous_filename].filter(Boolean)) {
65+
if (failures.some(failure => referencesFile(failure, filename))) {
66+
overlaps.add(filename);
67+
}
68+
}
69+
}
70+
if (overlaps.size) {
71+
cli.stopSpinner('Refusing to resume CI: failures reference files changed by this PR',
72+
cli.SPINNER_STATUS.FAILED);
73+
for (const filename of [...overlaps].sort()) {
74+
cli.error(filename);
75+
}
76+
return false;
77+
}
78+
cli.stopSpinner('No changed PR files found in available failure details');
79+
return true;
80+
}
81+
82+
async resume() {
83+
const { cli, request, prid } = this;
84+
let crumb;
85+
cli.startSpinner('Validating Jenkins credentials');
86+
try {
87+
({ crumb } = await request.json(CI_CRUMB_URL));
88+
if (!crumb) {
89+
throw new Error('Missing Jenkins crumb');
90+
}
91+
} catch (err) {
92+
debuglog(err);
93+
cli.stopSpinner('Jenkins credentials invalid', cli.SPINNER_STATUS.FAILED);
94+
return false;
95+
}
96+
cli.stopSpinner('Jenkins credentials valid');
97+
98+
try {
99+
cli.startSpinner(`Looking for CI runs for pull request ${prid}`);
100+
const parser = await JobParser.fromPR(getPrURL(this), cli, request);
101+
const job = parser.parse().get(CI_TYPES_KEYS.PR);
102+
if (!job) {
103+
cli.stopSpinner(`No CI run detected from pull request ${prid}`,
104+
cli.SPINNER_STATUS.FAILED);
105+
return false;
106+
}
107+
cli.stopSpinner(`Found PR CI job ${job.jobid}`);
108+
109+
const build = new PRBuild(cli, request, job.jobid, undefined,
110+
'result,building,actions[_class,parameters[name,value]]');
111+
const { result, building, actions = [] } = await build.getBuildData();
112+
if (building || (result !== 'FAILURE' && result !== 'ABORTED')) {
113+
const status = building ? 'RUNNING' : result ?? 'RUNNING';
114+
cli.error(`CI job ${job.jobid} is in status ${status}, skipping resume`);
115+
return false;
116+
}
117+
if (!actions.some(action =>
118+
action._class === 'com.tikal.jenkins.plugins.multijob.MultiJobResumeBuild')) {
119+
cli.error(`CI job ${job.jobid} is not resumable`);
120+
return false;
121+
}
122+
123+
const approvedSHAs = new Set(actions.flatMap(action => action.parameters ?? [])
124+
.filter(parameter => parameter.name === 'COMMIT_SHA_CHECK')
125+
.map(parameter => parameter.value));
126+
const [approvedSHA] = approvedSHAs;
127+
if (approvedSHAs.size !== 1 || typeof approvedSHA !== 'string' || !approvedSHA) {
128+
cli.error(`Refusing to resume CI job ${job.jobid}: cannot determine its approved commit`);
129+
return false;
130+
}
131+
132+
if (!(await this.checkFailures(job.jobid))) {
133+
return false;
134+
}
135+
136+
// Read HEAD after inspecting failures so the comparison is fresh when resuming.
137+
const pr = await request.getPullRequest(getPrURL(this));
138+
if (pr.head?.sha !== approvedSHA) {
139+
cli.error(`Refusing to resume CI job ${job.jobid}: ` +
140+
'its approved commit does not match the current PR HEAD');
141+
return false;
142+
}
143+
144+
cli.startSpinner(`Resuming PR CI job ${job.jobid}`);
145+
const response = await request.fetch(`${build.jobUrl}resume`, {
146+
method: 'POST',
147+
headers: {
148+
'Jenkins-Crumb': crumb
149+
}
150+
});
151+
if (response.status !== 200) {
152+
cli.stopSpinner(
153+
`Failed to resume PR CI: ${response.status} ${response.statusText}`,
154+
cli.SPINNER_STATUS.FAILED);
155+
return false;
156+
}
157+
cli.stopSpinner('PR CI job successfully resumed');
158+
} catch (err) {
159+
debuglog(err);
160+
cli.stopSpinner('Failed to resume CI', cli.SPINNER_STATUS.FAILED);
161+
return false;
162+
}
163+
return true;
164+
}
165+
}

‎lib/request.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,10 @@ export default class Request {
106106
const url =
107107
`/repos/${owner}/${repo}/pulls/${prid}/files?per_page=100&page=${page}`;
108108
const batch = await this.json(url);
109-
if (!Array.isArray(batch) || batch.length === 0) {
109+
if (!Array.isArray(batch)) {
110+
throw new Error('Unable to retrieve pull request files');
111+
}
112+
if (batch.length === 0) {
110113
break;
111114
}
112115
yield * batch;

‎test/fixtures/ci-resume-walk.json‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
[
2+
{
3+
"filename": "test/fixtures/typescript/ts/test-mock-module.ts",
4+
"failure": {
5+
"type": "JS_TEST_FAILURE",
6+
"file": "es-module/test-typescript",
7+
"url": "https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=0,nodes=win10-COMPILED_BY-vs2022_clang/43103/console",
8+
"upstream": "https://ci.nodejs.org/job/node-test-pull-request/77225/",
9+
"reason": "not ok 141 es-module/test-typescript\n '✖ c:\\\\workspace\\\\node-test-binary-windows-js-suites\\\\node\\\\test\\\\fixtures\\\\typescript\\\\ts\\\\test-mock-module.ts (45.1525ms)\\n' +"
10+
}
11+
},
12+
{
13+
"filename": "test/fixtures/rc/test.js",
14+
"failure": {
15+
"type": "JS_TEST_FAILURE",
16+
"file": "parallel/test-config-file",
17+
"url": "https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=0,nodes=win10-COMPILED_BY-vs2022_clang/43103/console",
18+
"upstream": "https://ci.nodejs.org/job/node-test-pull-request/77225/",
19+
"reason": "not ok 298 parallel/test-config-file\n + '✖ c:\\\\workspace\\\\node-test-binary-windows-js-suites\\\\node\\\\test\\\\fixtures\\\\rc\\\\test.js (106.6917ms)\\n' +"
20+
}
21+
},
22+
{
23+
"filename": "test/es-module/test-typescript.mjs",
24+
"failure": {
25+
"type": "JS_TEST_FAILURE",
26+
"file": "es-module/test-typescript",
27+
"url": "https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=0,nodes=win10-COMPILED_BY-vs2022_clang/43103/console",
28+
"upstream": "https://ci.nodejs.org/job/node-test-pull-request/77225/",
29+
"reason": "not ok 141 es-module/test-typescript\n Location: test\\es-module\\test-typescript.mjs:240:1"
30+
}
31+
},
32+
{
33+
"filename": "lib/internal/vfs/file_handle.js",
34+
"failure": {
35+
"type": "JS_TEST_FAILURE",
36+
"file": "/home/iojs/build/workspace/node-test-linter/lib/internal/vfs/file_handle.js",
37+
"url": "https://ci.nodejs.org/job/node-test-linter/67148/console",
38+
"upstream": "https://ci.nodejs.org/job/node-test-pull-request/77227/",
39+
"reason": "not ok 1056 - /home/iojs/build/workspace/node-test-linter/lib/internal/vfs/file_handle.js"
40+
}
41+
}
42+
]

0 commit comments

Comments
 (0)