Skip to content

ci: Fix automodel and submodule check comments from a fork#1028

Merged
terrykong merged 1 commit intoNVIDIA-NeMo:mainfrom
chtruong814:main
Sep 17, 2025
Merged

ci: Fix automodel and submodule check comments from a fork#1028
terrykong merged 1 commit intoNVIDIA-NeMo:mainfrom
chtruong814:main

Conversation

@chtruong814
Copy link
Contributor

What does this PR do ?

Fix automodel and submodule check comments from a fork

When a PR is made from a fork, the PR comment job does not have the right permissions to write to the PR due to security reasons. We can enable this by separating the PR comment steps to separate jobs that run on the pull_request_target event. When a job runs in the context of that event (as opposed to the typical pull_request event), it runs from the perspective off the base branch and has the permissions to write to the PR such as adding a comment.

https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target

Because of the increased privilege that the fork PR would have, it is not recommended to check out and do anything with the code from the PR itself. So, we only do the commenting with the pull_request_target event. To clarify:

  1. On pull_request the automodel and submodule checks happen as usual and write their comments to a Github artifact
  2. On pull_request_target the corresponding jobs poll for the artifacts to exist. If the artifacts do not exist, then nothing fails. If they do exist within a given number of tries, then it will comment on the PR.

Here's an example of this working:
chtruong814#3

Those comments are from a PR I made from my personal Github username to a fork of RL.

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

@chtruong814 chtruong814 requested a review from terrykong August 30, 2025 17:16
@github-actions github-actions bot added the CI Relating to CI label Aug 30, 2025
@chtruong814 chtruong814 requested a review from ko3n1g August 30, 2025 17:16
@chtruong814 chtruong814 changed the title Fix automodel and submodule check comments from a fork ci: Fix automodel and submodule check comments from a fork Aug 30, 2025
Signed-off-by: Charlie Truong <chtruong@nvidia.com>
@terrykong
Copy link
Collaborator

@ko3n1g to review

@terrykong terrykong enabled auto-merge September 16, 2025 17:06
@terrykong terrykong added this pull request to the merge queue Sep 16, 2025
Merged via the queue into NVIDIA-NeMo:main with commit 1cb5e0d Sep 17, 2025
21 checks passed
PrinsYin pushed a commit to PrinsYin/RL that referenced this pull request Nov 30, 2025
…Mo#1028)

Signed-off-by: Charlie Truong <chtruong@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Relating to CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants