Skip to content

ci: add NVSkills request workflow#719

Merged
johnnygreco merged 1 commit into
mainfrom
johnny/ci/request-nvskills-ci
May 29, 2026
Merged

ci: add NVSkills request workflow#719
johnnygreco merged 1 commit into
mainfrom
johnny/ci/request-nvskills-ci

Conversation

@johnnygreco
Copy link
Copy Markdown
Contributor

📋 Summary

Adds the NVSkills request workflow so maintainers can trigger NVSkills CI from pull request comments once this workflow exists on the default branch.

🔗 Related Issue

N/A

🔄 Changes

  • Add .github/workflows/request-nvskills-ci.yml from NVIDIA/skills.
  • Enable /nvskills-ci pull request comment requests and NVSkills signature push handling.

🧪 Testing

  • make test passes (N/A - workflow-only change)
  • Unit tests added/updated (N/A - no Python logic)
  • E2E tests added/updated (N/A - no app workflow)
  • Parsed workflow YAML with Ruby YAML.load_file

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (N/A - CI workflow only)

Signed-off-by: Johnny Greco <jogreco@nvidia.com>
@johnnygreco johnnygreco requested a review from a team as a code owner May 29, 2026 22:35
@johnnygreco johnnygreco deployed to agentic-ci May 29, 2026 22:36 — with GitHub Actions Active
@github-actions
Copy link
Copy Markdown
Contributor

Review: PR #719ci: add NVSkills request workflow

Summary

Adds a single new file, .github/workflows/request-nvskills-ci.yml (22 lines), that wires this repo into NVIDIA's shared NVSkills CI dispatcher. The workflow is a thin frontend over NVIDIA/skills/.github/workflows/team-request.yml@main and fires in two situations:

  1. A comment starting with /nvskills-ci is posted on a PR, or
  2. A push lands whose actor and commit title both match the configured signature attachment values (with sensible defaults).

The PR is workflow-only — no application/test code is touched. Scope and purpose match the description.

Findings

Correctness

  • Trigger gating looks right. Both branches of the if: are correctly guarded:
    • The issue_comment branch requires github.event.issue.pull_request (i.e. the comment is on a PR, not a bare issue) before checking the /nvskills-ci body prefix.
    • The push branch requires both the actor and the commit title to match, defaulting to nv-skills-ci[bot] and Attach NVSkills validation signatures when the repo vars are unset.
  • Folded scalar is fine. The > folded style on if: joins lines with spaces and preserves the boolean expression correctly. Confirmed by the author's manual YAML.load_file parse in the PR description.
  • startsWith semantics. startsWith(github.event.comment.body, '/nvskills-ci') will also match /nvskills-ci-anything or /nvskills-cisomething. If only the bare command (and /nvskills-ci <args>) should match, consider gating with a trailing space/EOL — but this matches the upstream NVIDIA/skills convention, so it's likely intentional.

Security

  • Pinned to @main, not a SHA. The reusable workflow NVIDIA/skills/.github/workflows/team-request.yml@main is referenced by branch. Floating refs on third-party reusable workflows are a supply-chain risk — a force-push or compromised maintainer on NVIDIA/skills would propagate here automatically. GitHub's own hardening guide recommends pinning to a full commit SHA. Counterpoint: this is a sibling NVIDIA org and the rest of the workflow ecosystem may already pin this way, so it may be intentional and consistent. Worth confirming with the org.
  • Top-level permissions are correctly minimal (contents: read, pull-requests: read). Any additional scopes the dispatcher needs are granted inside the called workflow, which is the right boundary.
  • issue_comment is a notoriously easy footgun. The classic risk is running untrusted PR-author code with elevated permissions; here the parent workflow does no checkout and no script execution — it only invokes a reusable workflow with a single secret. The risk surface is therefore confined to whatever the upstream team-request.yml does. Accepting this PR transfers trust to that workflow.
  • Secret passthrough. NVSKILLS_CI_DISPATCH_TOKEN is forwarded only when the gating if: already matched, so the secret is not exposed to arbitrary commenters/pushers. Good.

Style / Conventions

  • No pull_request paths/branches filter on push. The push: trigger has no branches: or paths: qualifiers, so it evaluates the if: on every push to every branch. That's cheap (the job is skipped when conditions don't match) and matches the apparent intent (signature-attachment commits can target any branch). Worth a brief comment in the file noting why the trigger is unbounded, since "push with no filter" is unusual in this repo's other workflows. Optional.
  • Trailing newline present (line 23). Good.
  • No name on the job beyond the job key request. Fine; consistent with shared-workflow callers.

Test coverage

N/A — workflow-only change. The author validated the YAML parses. Real validation requires the workflow to run on main (since reusable-workflow callers must exist on the default branch to be invokable from comments), so any latent issue will only be observable post-merge. That's an inherent limitation, not something to fix here.

Suggestions (all optional / low priority)

  1. Consider pinning the upstream reusable workflow to a SHA instead of @main (supply-chain hardening). If the rest of the NVIDIA org standardizes on @main, ignore.
  2. Add a one-line comment at the top explaining the two activation paths (comment trigger vs. signature push) — this is the kind of file that gets read once a year by someone debugging why CI didn't fire.
  3. Tighten the comment prefix match to /nvskills-ci followed by whitespace/EOL if you want to avoid accidental matches like /nvskills-cipher. Probably not worth it.

Verdict

Approve / no blockers. Small, focused, correctly gated CI plumbing. The only substantive concern is the @main pin on a third-party reusable workflow, and that is a project-wide policy question rather than a defect in this PR. Safe to merge as-is; the suggestions above are nice-to-haves.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR adds a new GitHub Actions workflow that enables NVSkills CI to be triggered from pull request comments (/nvskills-ci) and handles NVSkills signature pushes from the nv-skills-ci[bot] actor. The workflow is minimal and delegates all execution to the NVIDIA/skills reusable workflow, passing a single dispatch secret.

  • Comment trigger: Fires when an issue_comment is created on a PR whose body starts with /nvskills-ci; authorization is handled downstream by team-request.yml.
  • Push trigger: Fires on any push where the actor matches NVSKILLS_SIGNATURE_PUSH_ACTOR (or the default nv-skills-ci[bot]) and the commit message starts with the configured signature title — no branch filter is applied, which is appropriate since signatures can land on any ref.
  • Reusable workflow ref: The call to NVIDIA/skills/.github/workflows/team-request.yml@main is pinned to a mutable branch ref rather than a commit SHA, creating a soft supply-chain dependency on upstream changes.

Confidence Score: 4/5

Safe to merge; this is a thin wrapper workflow that delegates entirely to the upstream NVSkills infrastructure with minimal permissions.

The workflow is small and correct. The only notable concern is that the reusable workflow is referenced via a mutable @main branch rather than a pinned commit SHA — meaning any future push to NVIDIA/skills main automatically takes effect here, including the path that receives the NVSKILLS_CI_DISPATCH_TOKEN secret. This is a maintenance and supply-chain hygiene concern rather than a current defect.

.github/workflows/request-nvskills-ci.yml — specifically the @main ref on line 20.

Important Files Changed

Filename Overview
.github/workflows/request-nvskills-ci.yml New workflow enabling /nvskills-ci PR comment trigger and bot-push signature handling; delegates entirely to NVIDIA/skills reusable workflow pinned at @main (mutable ref)

Sequence Diagram

sequenceDiagram
    participant User as PR Commenter
    participant Bot as nv-skills-ci[bot]
    participant GHA as GitHub Actions
    participant NVS as NVIDIA/skills team-request.yml@main

    User->>GHA: Comment /nvskills-ci on PR
    GHA->>GHA: issue_comment trigger fires
    GHA->>NVS: "uses team-request.yml@main with NVSKILLS_CI_DISPATCH_TOKEN"

    Bot->>GHA: Push commit with signature message
    GHA->>GHA: push trigger fires, actor+message match
    GHA->>NVS: "uses team-request.yml@main with NVSKILLS_CI_DISPATCH_TOKEN"

    NVS-->>GHA: Dispatches NVSkills CI jobs
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/request-nvskills-ci.yml:20
**Mutable `@main` ref for reusable workflow**

The reusable workflow is pinned to `@main` rather than a specific commit SHA, so any change pushed to `NVIDIA/skills` main will take effect immediately in this repo without a review touchpoint. If the upstream workflow is inadvertently changed (or the repo is compromised), the `NVSKILLS_CI_DISPATCH_TOKEN` secret would be passed to the new version automatically. Pinning to a SHA (e.g. `NVIDIA/skills/.github/workflows/team-request.yml@<sha>`) with a periodic update process is the standard GitHub hardening recommendation for third-party reusable workflows.

Reviews (1): Last reviewed commit: "ci: add NVSkills request workflow" | Re-trigger Greptile

permissions:
contents: read
pull-requests: read
uses: NVIDIA/skills/.github/workflows/team-request.yml@main
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.

P2 Mutable @main ref for reusable workflow

The reusable workflow is pinned to @main rather than a specific commit SHA, so any change pushed to NVIDIA/skills main will take effect immediately in this repo without a review touchpoint. If the upstream workflow is inadvertently changed (or the repo is compromised), the NVSKILLS_CI_DISPATCH_TOKEN secret would be passed to the new version automatically. Pinning to a SHA (e.g. NVIDIA/skills/.github/workflows/team-request.yml@<sha>) with a periodic update process is the standard GitHub hardening recommendation for third-party reusable workflows.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/request-nvskills-ci.yml
Line: 20

Comment:
**Mutable `@main` ref for reusable workflow**

The reusable workflow is pinned to `@main` rather than a specific commit SHA, so any change pushed to `NVIDIA/skills` main will take effect immediately in this repo without a review touchpoint. If the upstream workflow is inadvertently changed (or the repo is compromised), the `NVSKILLS_CI_DISPATCH_TOKEN` secret would be passed to the new version automatically. Pinning to a SHA (e.g. `NVIDIA/skills/.github/workflows/team-request.yml@<sha>`) with a periodic update process is the standard GitHub hardening recommendation for third-party reusable workflows.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@johnnygreco johnnygreco merged commit 31b9f3c into main May 29, 2026
63 checks passed
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.

2 participants