Skip to content

feat: add ncu-ci resume command - #1187

Open
panva wants to merge 2 commits into
nodejs:mainfrom
panva:resume-ci
Open

feat: add ncu-ci resume command#1187
panva wants to merge 2 commits into
nodejs:mainfrom
panva:resume-ci

Conversation

@panva

@panva panva commented Sep 9, 2026

Copy link
Copy Markdown
Member

Reimplements #642 by @MoLow on current main, adding ncu-ci resume <prid> with these additions:

  • Accepts PR URLs as well as numeric IDs.
  • Handles missing CI links without crashing and rejects invalid PR IDs.
  • Rejects running jobs and results other than FAILURE or ABORTED. Checks Jenkins’ resume action, allowing resumable aborted jobs.
  • Refuses to resume when the CI-approved commit differs from the current PR HEAD, or cannot be determined.
  • Refuses when a failed test or failure diagnostic references a file changed by the PR. Handles renamed files, abbreviated test names, and literal or escaped Windows paths.
  • Streams failed-job logs one at a time with bounded memory, including for large individual lines and TAP blocks. Requests compressed responses and decompresses them while streaming.
  • Cancels the HTTP download when a match is confirmed and skips remaining logs.
  • Uses available failure details without blocking on unknown or unavailable diagnostics.

Includes command documentation, CLI exit-code tests, and regression fixtures captured with ncu-ci walk.

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: nodejs#642
Co-authored-by: Moshe Atlow <moshe@atlow.co.il>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.80233% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.45%. Comparing base (92bf51c) to head (638be4e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
lib/ci/failure_file_scanner.js 94.48% 7 Missing ⚠️
bin/ncu-ci.js 96.15% 2 Missing ⚠️
lib/ci/resume_ci.js 98.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1187      +/-   ##
==========================================
+ Coverage   57.81%   58.45%   +0.64%     
==========================================
  Files          53       59       +6     
  Lines       10396    11588    +1192     
==========================================
+ Hits         6010     6774     +764     
- Misses       4386     4814     +428     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread bin/ncu-ci.js
validate() {
const { cli, prid, repo, owner } = this;
let validArgs = true;
if (!Number.isSafeInteger(prid) || prid <= 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bold to assume that we won't have received 2^53 agentic PRs by the end of the year...

Comment on lines +5 to +6
const diagnostic = /error:|error C\d+:|Error:|ERROR:|fatal:|FATAL:|java\.io\.IOException|Build timed out|hudson\.plugins\.git\.GitException|Cannot rebase:|sh: line |dtrace: failed to compile script|Makefile:.+failed|make.*:.*(?:Error \d|write error)|warning: failed/;
const infrastructure = /Read-only file system|Device or resource busy|There is not enough space in the file system\./;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth modifying ci_failure_parser such that these can be extracted from there rather than duplicated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants