ci(skills): require NVSkills validation for skill changes - #147
Conversation
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
f688107 to
2e41be6
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a merge-blocking GitHub Actions check that enforces NVSkills validation for pull requests that modify skills/, and documents the maintainer/admin flow for triggering NVSkills CI and configuring branch protection.
Changes:
- Added a new PR workflow (
require-nvskills-ci.yml) that fails whenskills/changes are present but the PR HEAD is not the trusted NVSkills signature commit. - Documented the
/nvskills-ciflow and the required branch protection check in.github/CI_README.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/require-nvskills-ci.yml |
Adds the new required check that validates the PR HEAD commit matches the trusted NVSkills signature commit when skills/ files change. |
.github/CI_README.md |
Documents the NVSkills CI request workflow and the new required NVSkills enforcement workflow and branch protection requirements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ed85df5 to
2e41be6
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a merge-blocking GitHub Actions workflow ( ChangesNVSkills CI Validation
Sequence DiagramsequenceDiagram
participant PR as PullRequest
participant Actions as GitHubActions
participant GHAPI as GitHubAPI
PR->>Actions: PR opened or synchronized
Actions->>GHAPI: list changed files
GHAPI->>Actions: return file list (skills/ detected)
Actions->>GHAPI: get PR head commit
GHAPI->>Actions: return commit author and message
Actions->>Actions: validate actor == SIGNATURE_ACTOR and title startsWith SIGNATURE_TITLE
Actions-->>PR: set check success or failure
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/require-nvskills-ci.yml:
- Line 17: The workflow is using the unpinned action reference "uses:
actions/github-script@v7"; replace this with a specific commit SHA to prevent
supply-chain breakage by locating the line with uses: actions/github-script@v7
(and the other occurrence in docs.yaml) and change it to uses:
actions/github-script@<commit-sha> where <commit-sha> is the full 40-character
commit hash from the actions/github-script repository (fetch the current
recommended pinned SHA from the action's repo/tags) so both workflows use the
same immutable ref.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 186fe18f-fc35-418e-a8ef-d21673ebbf85
📒 Files selected for processing (2)
.github/CI_README.md.github/workflows/require-nvskills-ci.yml
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/.nvskills-ci-signature-test.md`:
- Line 1: The file's first line must be a top-level heading to satisfy MD041;
replace or prepend the current first line "Temporary NVSkills CI signature test
file." with an H1 heading (e.g., "# Temporary NVSkills CI signature test file.")
so the document begins with a single leading "#" heading on line 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2bcbc78e-634e-4d50-a26c-44a0d7065249
📒 Files selected for processing (1)
skills/.nvskills-ci-signature-test.md
|
/nvskills-ci |
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
|
/nvskills-ci |
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
|
/nvskills-ci |
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Summary
skills/changes/nvskills-cimaintainer/admin flow and required branch protection checkLinear
https://linear.app/nvidia/issue/AALGO-233/block-merges-when-skills-changes-lack-nvskill-pipeline-validation
Validation
/nvskills-ci---> triggerred nvskills pipeline ---> successful nvskills pipeline committed sig file ---> auto-triggeredRequire NVSkills CIpasseduv run --frozen python -c "from pathlib import Path; import yaml; yaml.safe_load(Path('.github/workflows/require-nvskills-ci.yml').read_text()); print('yaml ok')"git diff --checkNotes
actionlintwas not installed locally, so it was not run.Require NVSkills CI for skill changes / require-nvskills-ciin branch protection or rulesets.Summary by CodeRabbit
Documentation
Chores