Skip to content

ci: resolve PR number from head repo/branch for fork PRs - #1041

Open
btea wants to merge 4 commits into
nodejs:mainfrom
btea:chore/local-comment-pr-action
Open

ci: resolve PR number from head repo/branch for fork PRs #1041
btea wants to merge 4 commits into
nodejs:mainfrom
btea:chore/local-comment-pr-action

Conversation

@btea

@btea btea commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Description

The "Leave a comment" workflow fails because thollander/actions-comment-pull-request runs on the deprecated Node 20 runtime [1], and the upstream PR upgrading it to Node 24 [2] has been open without activity since March 2026.

Replace the action with a local composite action built on actions/github-script. The marker format is kept so existing
tagged comments are updated instead of duplicated.

[1] https://github.com/nodejs/doc-kit/actions/runs/31868656146/job/94973630400
[2] thollander/actions-comment-pull-request#449

Validation

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run node --run test and all tests passed.
  • I have check code formatting with node --run format:check & node --run lint.
  • I've covered new added functionality with unit tests if necessary.

@btea
btea requested a review from a team as a code owner August 15, 2026 06:35
@vercel

vercel Bot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-docs-tooling Ready Ready Preview Aug 15, 2026 3:33pm

Request Review

@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.46%. Comparing base (15b3f3e) to head (5cfcc33).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1041      +/-   ##
==========================================
+ Coverage   89.37%   89.46%   +0.09%     
==========================================
  Files         203      203              
  Lines       18986    19039      +53     
  Branches     1769     1782      +13     
==========================================
+ Hits        16968    17033      +65     
+ Misses       2011     1999      -12     
  Partials        7        7              

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MattIPv4 MattIPv4 left a comment

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.

I don't think Node.js 20/24 has anything to do with that failure -- the workflow was triggered from a commit on main, and so github.event.workflow_run.pull_requests[0].number used as the input will not be populated.

@btea

btea commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

You're right, I didn't address the error; I simply removed the warning message about the Node version. I've rewritten the logic, but it doesn't seem to be directly verifiable in this PR.

@avivkeller avivkeller left a comment

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.

I'm also wary to replace something that works, if it ain't broke don't fix it... why do we need our own logic when an external action works fine

@btea

btea commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

The local logic was added to resolve a Node.js version compatibility warning. The upstream action hasn't been updated in a long time, suggesting the author is no longer managing it. If you believe this warning can be ignored, I can revert the code and continue using the upstream action.

image

@MattIPv4

Copy link
Copy Markdown
Member

👍 Yes, no need to introduce additional maintenance burden by rolling our own action, the upstream works w/ Node.js 24.

@btea btea changed the title ci: replace third-party comment action with local one ci: ci: resolve PR number from head repo/branch for fork PRs Aug 15, 2026

@MattIPv4 MattIPv4 left a comment

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.

Can we follow the same pattern as in https://github.com/nodejs/nodejs.org/blob/main/.github/workflows/leave-comment.yml, please, so we have consistent implementations?

@btea btea changed the title ci: ci: resolve PR number from head repo/branch for fork PRs ci: resolve PR number from head repo/branch for fork PRs Aug 15, 2026
@btea

btea commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

It turns out there's a well-established logic to refer to; thank you for pointing that out.

@MattIPv4

Copy link
Copy Markdown
Member

@btea could I ask if you're using AI for your contributions and/or your comments?

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