Skip to content

feat: add ncu-ci resume command - #1187

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

feat: add ncu-ci resume command#1187
panva wants to merge 3 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.

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.82122% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.03%. Comparing base (d4bbaeb) to head (a8becd3).

Files with missing lines Patch % Lines
bin/ncu-ci.js 96.15% 2 Missing ⚠️
lib/ci/failure_file_scanner.js 98.69% 2 Missing ⚠️
lib/ci/resume_ci.js 98.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1187      +/-   ##
==========================================
+ Coverage   58.13%   59.03%   +0.89%     
==========================================
  Files          53       59       +6     
  Lines       10406    11644    +1238     
==========================================
+ Hits         6050     6874     +824     
- Misses       4356     4770     +414     

☔ 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
Comment thread lib/ci/failure_file_scanner.js Outdated
Comment thread docs/ncu-ci.md
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
determined.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW Jenkins already checks for that (for the HEAD, not for approvals). Just saying in case that adds too much complexity

@panva panva Sep 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, but that check runs inside the build script, so we'd get a resume link followed by a failed build, rather than [resume-ci-failed] on the PR, which is what I was aiming for.

Comment thread docs/ncu-ci.md Outdated
Comment thread lib/ci/resume_ci.js Outdated
Comment thread lib/ci/resume_ci.js
panva and others added 2 commits September 10, 2026 11:03
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
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.

4 participants