Skip to content

ci(nvskills): add NVSkills CI dispatch workflow - #9998

Merged
dagil-nvidia merged 3 commits into
mainfrom
claude/funny-swartz-d4bfae
May 27, 2026
Merged

ci(nvskills): add NVSkills CI dispatch workflow#9998
dagil-nvidia merged 3 commits into
mainfrom
claude/funny-swartz-d4bfae

Conversation

@dagil-nvidia

@dagil-nvidia dagil-nvidia commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Vendors NVIDIA/skills/.github/workflows/team-request.yml as .github/workflows/nvskills-team-request.yml (reusable workflow, watched paths kept identical to the upstream-supported set: skills/, team-skills/, rules/team-rules/, plugins/).
  • Adds .github/workflows/nvskills-ci.yml as a thin caller that fires on issue_comment (created) and push, invoking the reusable workflow and passing NVSKILLS_CI_DISPATCH_TOKEN through.
  • Repo secret NVSKILLS_CI_DISPATCH_TOKEN has already been set on ai-dynamo/dynamo.

Notes

  • The /nvskills-ci PR-comment trigger only fires when the workflow file is on the default branch, so the comment dispatch path won't be live until this PR is merged to main. The push signature-commit path will work on this branch once merged.
  • None of the upstream-watched paths (skills/, team-skills/, rules/team-rules/, plugins/) exist in this repo yet, so until they're added (e.g. via the .agents/skills/ canonical layout), every run skips with the "no changes" summary.
  • push: is currently unscoped; the job-level if: filters to the bot's signature commit. If the queued-but-skipped runs are noisy we can scope by branches: later.

Test plan

  • Merge to main and confirm the workflow appears in Actions on ai-dynamo/dynamo.
  • Once a PR touches one of the watched paths, comment /nvskills-ci from a maintainer/admin account and verify a dispatch is sent to NVIDIA/nvskills-ci/.github/workflows/nvskills-ci.yml.
  • Verify the bot signature-push path (nv-nvskill-ci[bot] pushing the Attach NVSkills validation signatures commit) successfully dispatches.

🤖 Generated with Claude Code


Open in Devin Review

Summary by CodeRabbit

  • Chores
    • Updated internal CI/CD automation configuration to enhance workflow integration and security controls for automated processes.

Review Change Stack

Vendors the NVCARPS/NVSkills team-request reusable workflow from
NVIDIA/skills and wires a thin caller so PR comments (`/nvskills-ci`)
and bot signature commits in this repo can dispatch NVSkills CI on
NVIDIA/nvskills-ci. Watched paths match the upstream-supported set
(`skills/`, `team-skills/`, `rules/team-rules/`, `plugins/`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia requested a review from a team as a code owner May 26, 2026 23:45
@github-actions github-actions Bot added ci Issues/PRs that reference CI build/test actions labels May 26, 2026
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Two new GitHub Actions workflows add conditional NVSkills CI dispatch: nvskills-ci.yml triggers on push and issue_comment events with minimal permissions, delegating to nvskills-team-request.yml, which validates requester permissions, resolves PR context, checks for changes in watched directories, and conditionally dispatches to the external NVIDIA/nvskills-ci workflow.

Changes

NVSkills CI Workflow Integration

Layer / File(s) Summary
Workflow entry point and reusable setup
.github/workflows/nvskills-ci.yml, .github/workflows/nvskills-team-request.yml
nvskills-ci.yml defines triggers (push, issue_comment) and permissions (contents:read, pull-requests:read), calling nvskills-team-request.yml as a reusable workflow. nvskills-team-request.yml header establishes workflow_call input requirements and a conditionally-gated job for issue_comment and push signature events with concurrency control.
Request validation gating
.github/workflows/nvskills-team-request.yml
Permission validation step uses GitHub API to verify that issue_comment authors hold admin or maintain repository permission before proceeding.
PR context and file change detection
.github/workflows/nvskills-team-request.yml
Context resolution derives PR metadata (number, SHA, base ref, commit title), validates push event signatures (commit message and actor match NVSkills values), pages PR file lists to detect changes in watched directories (skills/, team-skills/, rules/team-rules/, plugins/), and sets dispatch decision flag.
Conditional dispatch to external workflow
.github/workflows/nvskills-team-request.yml
Dispatch step validates secret presence and calls NVIDIA/nvskills-ci workflow dispatch API with repository, PR, and request metadata when dispatch is approved.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding NVSkills CI dispatch workflows. It is concise, specific, and clearly describes the primary purpose of the changeset.
Description check ✅ Passed The description covers all required template sections: Overview/Summary explaining the changes, Details about what was added, direction on where to start (notes on behavior), and related information. However, the Related Issues section is missing a specific issue reference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (1)
.github/workflows/nvskills-team-request.yml (1)

138-144: 💤 Low value

Consider using heredoc syntax for commit_title output.

If a commit title contains %, =, or other characters that GitHub Actions interprets specially in output strings, the value could be truncated or malformed. Using heredoc syntax is more robust:

{
  echo "should_dispatch=true"
  echo "pr_number=${pr_number}"
  echo "head_sha=${head_sha}"
  echo "base_ref=${base_ref}"
  echo 'commit_title<<EOF'
  printf '%s' "${commit_title}"
  echo
  echo 'EOF'
} >> "${GITHUB_OUTPUT}"

That said, the current approach is likely fine for typical commit titles.

🤖 Prompt for 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.

In @.github/workflows/nvskills-team-request.yml around lines 138 - 144, The
current block that appends variables to "${GITHUB_OUTPUT}" can break if
commit_title contains special chars; change the way commit_title is written by
using heredoc syntax: keep echo lines for should_dispatch, pr_number, head_sha,
base_ref as-is, then write a heredoc marker like 'commit_title<<EOF' to
GITHUB_OUTPUT, emit the raw commit_title via printf '%s' "${commit_title}"
(followed by a newline) and close the heredoc with EOF before redirecting,
ensuring commit_title is preserved; reference the variables commit_title,
pr_number, head_sha, base_ref and the output target "${GITHUB_OUTPUT}" when
making the change.
🤖 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.

Nitpick comments:
In @.github/workflows/nvskills-team-request.yml:
- Around line 138-144: The current block that appends variables to
"${GITHUB_OUTPUT}" can break if commit_title contains special chars; change the
way commit_title is written by using heredoc syntax: keep echo lines for
should_dispatch, pr_number, head_sha, base_ref as-is, then write a heredoc
marker like 'commit_title<<EOF' to GITHUB_OUTPUT, emit the raw commit_title via
printf '%s' "${commit_title}" (followed by a newline) and close the heredoc with
EOF before redirecting, ensuring commit_title is preserved; reference the
variables commit_title, pr_number, head_sha, base_ref and the output target
"${GITHUB_OUTPUT}" when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 08a61852-b98d-48a8-adae-b9239bf6419e

📥 Commits

Reviewing files that changed from the base of the PR and between 936d64f and b2a37f7.

📒 Files selected for processing (2)
  • .github/workflows/nvskills-ci.yml
  • .github/workflows/nvskills-team-request.yml

@dmitry-tokarev-nv dmitry-tokarev-nv left a comment

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.

Review summary

Reviewed the vendored reusable workflow and the thin caller. Design and security gates look reasonable; the main blocker is missing SPDX headers, which will fail the copyright-checks job. A few should-fix and nit-level items are left inline.

Criticality legend

  • Blocker — must address before merge (will fail CI or break expected behavior).
  • Should fix — correctness/robustness gap; address before merge unless explicitly deferred.
  • Nit — cosmetic, optional hardening, or stylistic.

Findings

Blockers

  • Missing SPDX header on both new workflow files (copyright-check.ps1 enforces .yml/.yaml). Inline suggestions provided.

Should fix

  • Unbounded pagination loop in Resolve request context (nvskills-team-request.yml:107). Inline suggestion bounds it at 30 pages.
  • Push-path trust model relies only on github.actor + commit title — no branch scoping and no verification that the bot push went through branch protection. Consider scoping push: by branches: (PR description already flags this) and confirm the dispatch PAT is minimally scoped on NVIDIA/nvskills-ci.

Nits

  • startsWith(github.event.comment.body, '/nvskills-ci') also matches /nvskills-ci-anything. Inline suggestion tightens to an anchored token (or exact match).
  • cancel-in-progress: true on the push path can kill an in-flight dispatch if the bot rapidly re-pushes its signature commit. Inline note.
  • Caller (nvskills-ci.yml) has no if: short-circuit, so every issue_comment and push reaches the reusable workflow. Cheap, but trivially avoidable.
  • Vendored from NVIDIA/skills/.github/workflows/team-request.yml; a # Vendored from <repo>@<sha> header would aid future syncs.

Notes (not actionable)

  • Token hygiene is good: github.token (read-only) for API reads, dedicated NVSKILLS_CI_DISPATCH_TOKEN for cross-repo dispatch, minimal workflow-level permissions.
  • Permission gate on the issue_comment path correctly requires admin|maintain.
  • issue_comment: [created] (not edited) correctly prevents replay via comment edit.
  • No third-party actions; only local reusable workflow + bash run: steps. Matches the repo's enterprise allowlist.
  • Commit is DCO-signed; GPG sig shows E (unverified key) — non-blocking but flag for visibility.

Comment thread .github/workflows/nvskills-ci.yml
Comment thread .github/workflows/nvskills-team-request.yml
Comment thread .github/workflows/nvskills-team-request.yml Outdated
Comment thread .github/workflows/nvskills-team-request.yml Outdated
Comment thread .github/workflows/nvskills-team-request.yml Outdated
Comment thread .github/workflows/nvskills-team-request.yml
- Add SPDX headers on both workflow files to satisfy copyright-checks
  and match repo convention.
- Note the upstream source/SHA in team-request.yml for future syncs.
- Tighten /nvskills-ci comment match to anchored token (no prefix
  bleed into /nvskills-cix).
- Make cancel-in-progress conditional on issue_comment so back-to-back
  bot signature pushes aren't aborted mid-dispatch; split concurrency
  group by event_name.
- Bound the PR-files pagination loop at GitHub's 3000-file (30 page)
  cap so a misbehaving API can't loop indefinitely.

Push-path branch scoping and dispatch-token-scope confirmation
remain deferred follow-ups, as noted in the PR description.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia enabled auto-merge (squash) May 27, 2026 00:57
NVIDIA/skills declares `SPDX-License-Identifier: Apache-2.0 AND
CC-BY-4.0` in its LICENSE file (Apache-2.0 for code/configuration,
CC-BY-4.0 for documentation). Reflect the same dual identifier on
the vendored files instead of Apache-2.0 alone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia merged commit a193497 into main May 27, 2026
100 checks passed
@dagil-nvidia
dagil-nvidia deleted the claude/funny-swartz-d4bfae branch May 27, 2026 01:47
dagil-nvidia added a commit that referenced this pull request May 27, 2026
NVIDIA/nvskills-ci's onboarded-repositories.json policy expects request
workflows at the canonical path .github/workflows/request-nvskills-ci.yml.
PR #9998 landed our wrapper as .github/workflows/nvskills-ci.yml, which
collides with the destination workflow's name and is rejected by the
downstream validator with:

    Request workflow path '.github/workflows/nvskills-ci.yml' is not allowed

Rename the wrapper to request-nvskills-ci.yml and update the in-workflow
'name:' from "NVSkills CI" to "Request NVSkills CI" — matches the upstream
template at NVIDIA/nvskills-ci/templates/team-request-workflow.yml and
disambiguates from the validation workflow on the other side.

No behavior change. Same triggers, same secret, same callee.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

actions ci Issues/PRs that reference CI build/test size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants