Skip to content

chore(ci): install NVSkills CI request listener for /nvskills-ci#4345

Merged
jyaunches merged 2 commits into
mainfrom
chore/4282-install-nvskills-ci-listener
May 27, 2026
Merged

chore(ci): install NVSkills CI request listener for /nvskills-ci#4345
jyaunches merged 2 commits into
mainfrom
chore/4282-install-nvskills-ci-listener

Conversation

@jyaunches
Copy link
Copy Markdown
Contributor

@jyaunches jyaunches commented May 27, 2026

Summary

Install the NVSkills CI request listener so /nvskills-ci comments on catalog refresh PRs actually trigger NVSkills signing. Without this listener, comments are no-ops and merged refresh PRs ship unsigned (which downstream NVIDIA/skills then drops on sync).

Related Issue

Follow-up to #4282 / #4342. Discovered while running the post-merge signing flow on PR #4344: the /nvskills-ci comment posted on that PR produced no workflow run because NemoClaw was missing the team-request listener.

Changes

  • .github/workflows/request-nvskills-ci.yml (new): copied byte-for-byte from NVIDIA/nvskills-ci/templates/team-request-workflow.yml@main, with a NemoClaw SPDX header. Forwards /nvskills-ci PR comments (and nv-skills-ci[bot] signature pushes) to NVIDIA/skills/.github/workflows/team-request.yml@main via secrets.NVSKILLS_CI_DISPATCH_TOKEN.
  • .github/CODEOWNERS: add an explicit nemoclaw-maintainer rule for /.github/workflows/request-nvskills-ci.yml so onboarding step 4 (CODEOWNERS protection) is visibly enforced; also flatten the now-stale /skills/nemoclaw/ rule to /skills/ to match the post-fix(skills): flatten catalog export and detect untracked files in refresh diff #4342 export layout.

Onboarding status

Per NVIDIA/nvskills-ci/docs/team-onboarding.md:

Step What Status
1 Add NemoClaw to config/onboarded-repositories.json ✅ already done
2 Install templates/team-request-workflow.yml as .github/workflows/request-nvskills-ci.yml ✅ this PR
3 Set NVSKILLS_CI_DISPATCH_TOKEN repo secret ⚠️ manual maintainer/admin action required after merge
4 CODEOWNERS-protect the new workflow file ✅ this PR
5 Test by commenting /nvskills-ci on a PR 🟡 unblocked once step 3 lands

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • Workflow body verified byte-identical to upstream template:

    $ diff <(tail -n +13 .github/workflows/request-nvskills-ci.yml) \
           ~/Development/nvskills-ci/templates/team-request-workflow.yml
    (no output — identical)
    
  • Listener is inert until the NVSKILLS_CI_DISPATCH_TOKEN secret is set; until then /nvskills-ci comments will fail at the secret-injection boundary, but they will at least produce a visible failed workflow run instead of silently doing nothing.

  • No secrets, API keys, or credentials committed

  • Tests added or updated for new or changed behavior — N/A (single workflow file mirrored from upstream template; behavior is exercised by the next live /nvskills-ci comment)

  • Docs updated for user-facing behavior changes — N/A (CI-internal)

  • npm run docs builds without warnings — N/A

  • Doc pages follow the style guide — N/A

  • New doc pages include SPDX header and frontmatter — N/A


Signed-off-by: Justin Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Chores
    • Updated repository ownership mappings to reorganize responsibility for the skills subtree and simplify maintainer coverage.
    • Added a workflow to request NVSkills validation/signature runs: can be triggered by a specially formatted issue comment or by specific commit pushes from the CI signature actor, and securely forwards the dispatch token to the centralized validation workflow.

Review Change Stack

Adds the team-request workflow from NVIDIA/nvskills-ci so PR comments of
`/nvskills-ci` (and signature push events from `nv-skills-ci[bot]`) are
forwarded to the central NVSkills CI service. Without this listener,
`/nvskills-ci` comments on the catalog refresh PR (#4344 and successors)
are no-ops and signing artifacts (`skill.oms.sig`, `skill-card.md`) are
never pushed back, leaving merged refresh PRs unsigned.

Completes NVIDIA/nvskills-ci team-onboarding steps 2 and 4 for NemoClaw:

- `.github/workflows/request-nvskills-ci.yml`: copied byte-for-byte from
  templates/team-request-workflow.yml@main, with a NemoClaw SPDX header.
  Triggers `NVIDIA/skills/.github/workflows/team-request.yml@main` via
  `secrets.NVSKILLS_CI_DISPATCH_TOKEN`.
- `.github/CODEOWNERS`: explicitly require nemoclaw-maintainer review on
  the new workflow file (CODEOWNERS protection is required by onboarding
  step 4); also fix the now-stale `/skills/nemoclaw/` rule to `/skills/`
  to match the flattened export layout from #4342.

Onboarding step 3 (set the `NVSKILLS_CI_DISPATCH_TOKEN` repo secret)
must be completed by a maintainer/admin via repo settings before the
listener can dispatch successfully. Until that secret is set, runs of
this workflow will fail at the secrets-injection boundary.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches jyaunches added the v0.0.53 Release target label May 27, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 74b743d7-bd06-43ce-b5e4-29b24c286ee3

📥 Commits

Reviewing files that changed from the base of the PR and between 889c4ee and ccee74e.

📒 Files selected for processing (1)
  • .github/workflows/request-nvskills-ci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/request-nvskills-ci.yml

📝 Walkthrough

Walkthrough

Broadens CODEOWNERS to explicitly cover /skills/ and adds a Request NVSkills CI GitHub Actions workflow that dispatches NVSkills validation requests (on /nvskills-ci PR comments or signature pushes) to a pinned NVIDIA/skills reusable workflow.

Changes

NVSkills CI Integration

Layer / File(s) Summary
CODEOWNERS /skills/ directory assignment
.github/CODEOWNERS
Replaces the narrower /.agents/skills/nemoclaw/ CODEOWNERS rule with an explicit /skills/ path assignment for @NVIDIA/nemoclaw-maintainer and @NVIDIA/nemoclaw-engineer.
NVSkills CI request workflow
.github/workflows/request-nvskills-ci.yml
New workflow Request NVSkills CI triggers on /nvskills-ci PR comments or signature-related pushes and dispatches NVIDIA/skills/.github/workflows/team-request.yml@<pinned-sha> with NVSKILLS_CI_DISPATCH_TOKEN and read-only permissions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4334: Adds catalog refresh workflow that uses /nvskills-ci to trigger NVSkills signing, which pairs with this PR's dispatch workflow.
  • NVIDIA/NemoClaw#4284: Related to catalog export automation that may post /nvskills-ci commands, integrating with this PR's request handler.

Suggested labels

CI/CD

Suggested reviewers

  • miyoungc
  • cv

Poem

🐇 A small command whispered low,
/nvskills-ci set to go,
The /skills/ path now assigned,
A token sent, the call aligned,
Validation hops — ready, go!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: installing an NVSkills CI request listener for the /nvskills-ci command.
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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/4282-install-nvskills-ci-listener

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No NemoClaw E2E is recommended. The PR only changes GitHub repository governance/CI dispatch configuration and does not affect runtime code paths, installer/onboarding behavior, sandbox lifecycle, credential migration/sanitization, network policy, inference routing, deployment logic, or real assistant workflows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

PR Review Advisor

Findings: 1 needs attention, 4 worth checking, 0 nice ideas
Since last review: 1 prior item resolved, 4 still apply, 0 new items found

Review findings

🛠️ Needs attention

  • Authorize /nvskills-ci before passing the dispatch token (.github/workflows/request-nvskills-ci.yml:25): Any newly created PR comment whose body starts with `/nvskills-ci` satisfies the local gate and invokes the reusable workflow with `NVSKILLS_CI_DISPATCH_TOKEN`. The workflow does not check the comment author's repository role, team membership, author association, or effective permission before crossing the secret-bearing trust boundary. The prefix-only parse also accepts unexpected command shapes such as `/nvskills-ci-anything` or `/nvskills-ci extra`.
    • Recommendation: Add an explicit authorization gate before the secret is made available. For example, restrict accepted comments to trusted author associations such as OWNER/MEMBER/COLLABORATOR if that matches the process, or add a preliminary permission-check job/API call and only invoke the reusable workflow when the commenter is authorized. Match an exact command shape unless the downstream workflow has a reviewed argument contract.
    • Evidence: `issue_comment` events are accepted when `github.event.issue.pull_request` is true and `startsWith(github.event.comment.body, '/nvskills-ci')`; the job then calls `NVIDIA/skills/.github/workflows/team-request.yml@d6c3af084edd278e5524c95e6586fc763835eb62` and passes `secrets.NVSKILLS_CI_DISPATCH_TOKEN`. The local signing-flow docs describe the signing request as a maintainer-controlled handoff.

🔎 Worth checking

  • Source-of-truth review needed: .github/workflows/request-nvskills-ci.yml mirrored NVSkills listener: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The workflow header says it mirrors `templates/team-request-workflow.yml from NVIDIA/nvskills-ci@main`, while the job calls a pinned `NVIDIA/skills/.github/workflows/team-request.yml@d6c3af084edd278e5524c95e6586fc763835eb62` and no sync/update policy is added.
  • Add negative coverage for the workflow command gate (.github/workflows/request-nvskills-ci.yml:25): This PR adds a privileged workflow-command surface but no automated policy or regression coverage for who may trigger it, which command forms are accepted, or whether the secret-bearing reusable workflow remains pinned.
    • Recommendation: Add a lightweight workflow-policy test or static check that asserts unauthorized/public PR commenters cannot reach the secret-bearing reusable workflow, non-PR issue comments are ignored, unexpected command shapes are handled intentionally, push-trigger gates are constrained, and the reusable workflow ref remains pinned.
    • Evidence: Only `.github/CODEOWNERS` and `.github/workflows/request-nvskills-ci.yml` changed; no tests or policy checks were added. The deterministic test-depth context says the changed workflow logic can be covered with unit/static tests.
  • Make the workflow-specific CODEOWNERS rule the effective match (.github/CODEOWNERS:40): The PR adds an explicit CODEOWNERS entry for `/.github/workflows/request-nvskills-ci.yml`, but CODEOWNERS uses last-match-wins and the later `/.github/` rule is currently the effective match. Today both rules name `@NVIDIA/nemoclaw-maintainer`, so protection is equivalent, but the explicit listener rule is not the durable source of truth the comment claims.
    • Recommendation: Move the listener-specific rule below the broader `/.github/` rule, or otherwise structure the CODEOWNERS entries so the listener-specific protection remains the effective last match if broader `.github` ownership changes later.
    • Evidence: Line 40 adds `/.github/workflows/request-nvskills-ci.yml @NVIDIA/nemoclaw-maintainer`, while the later `/.github/ @NVIDIA/nemoclaw-maintainer` line matches the same file after it.
  • Define the sync contract for the mirrored NVSkills listener (.github/workflows/request-nvskills-ci.yml:4): The workflow comments say this file mirrors an upstream NVSkills template, but the PR does not add a local source-of-truth guard, update process, or removal condition. That makes it easy for the local listener to drift from the reviewed upstream template or from the pinned reusable workflow contract.
    • Recommendation: Document how the mirrored listener should be updated, including how the pinned reusable-workflow SHA is selected and reviewed. Add a static check or snapshot policy that catches accidental drift in the local listener and the pinned `uses:` ref.
    • Evidence: The workflow says it `Mirrors templates/team-request-workflow.yml from NVIDIA/nvskills-ci@main`; the called reusable workflow is pinned to `d6c3af084edd278e5524c95e6586fc763835eb62`, but there is no sync/pinning regression check or update note in the changed files.

🌱 Nice ideas

  • None.
Since last review details

Current findings:

  • Source-of-truth review needed: .github/workflows/request-nvskills-ci.yml mirrored NVSkills listener: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The workflow header says it mirrors `templates/team-request-workflow.yml from NVIDIA/nvskills-ci@main`, while the job calls a pinned `NVIDIA/skills/.github/workflows/team-request.yml@d6c3af084edd278e5524c95e6586fc763835eb62` and no sync/update policy is added.
  • Authorize /nvskills-ci before passing the dispatch token (.github/workflows/request-nvskills-ci.yml:25): Any newly created PR comment whose body starts with `/nvskills-ci` satisfies the local gate and invokes the reusable workflow with `NVSKILLS_CI_DISPATCH_TOKEN`. The workflow does not check the comment author's repository role, team membership, author association, or effective permission before crossing the secret-bearing trust boundary. The prefix-only parse also accepts unexpected command shapes such as `/nvskills-ci-anything` or `/nvskills-ci extra`.
    • Recommendation: Add an explicit authorization gate before the secret is made available. For example, restrict accepted comments to trusted author associations such as OWNER/MEMBER/COLLABORATOR if that matches the process, or add a preliminary permission-check job/API call and only invoke the reusable workflow when the commenter is authorized. Match an exact command shape unless the downstream workflow has a reviewed argument contract.
    • Evidence: `issue_comment` events are accepted when `github.event.issue.pull_request` is true and `startsWith(github.event.comment.body, '/nvskills-ci')`; the job then calls `NVIDIA/skills/.github/workflows/team-request.yml@d6c3af084edd278e5524c95e6586fc763835eb62` and passes `secrets.NVSKILLS_CI_DISPATCH_TOKEN`. The local signing-flow docs describe the signing request as a maintainer-controlled handoff.
  • Add negative coverage for the workflow command gate (.github/workflows/request-nvskills-ci.yml:25): This PR adds a privileged workflow-command surface but no automated policy or regression coverage for who may trigger it, which command forms are accepted, or whether the secret-bearing reusable workflow remains pinned.
    • Recommendation: Add a lightweight workflow-policy test or static check that asserts unauthorized/public PR commenters cannot reach the secret-bearing reusable workflow, non-PR issue comments are ignored, unexpected command shapes are handled intentionally, push-trigger gates are constrained, and the reusable workflow ref remains pinned.
    • Evidence: Only `.github/CODEOWNERS` and `.github/workflows/request-nvskills-ci.yml` changed; no tests or policy checks were added. The deterministic test-depth context says the changed workflow logic can be covered with unit/static tests.
  • Make the workflow-specific CODEOWNERS rule the effective match (.github/CODEOWNERS:40): The PR adds an explicit CODEOWNERS entry for `/.github/workflows/request-nvskills-ci.yml`, but CODEOWNERS uses last-match-wins and the later `/.github/` rule is currently the effective match. Today both rules name `@NVIDIA/nemoclaw-maintainer`, so protection is equivalent, but the explicit listener rule is not the durable source of truth the comment claims.
    • Recommendation: Move the listener-specific rule below the broader `/.github/` rule, or otherwise structure the CODEOWNERS entries so the listener-specific protection remains the effective last match if broader `.github` ownership changes later.
    • Evidence: Line 40 adds `/.github/workflows/request-nvskills-ci.yml @NVIDIA/nemoclaw-maintainer`, while the later `/.github/ @NVIDIA/nemoclaw-maintainer` line matches the same file after it.
  • Define the sync contract for the mirrored NVSkills listener (.github/workflows/request-nvskills-ci.yml:4): The workflow comments say this file mirrors an upstream NVSkills template, but the PR does not add a local source-of-truth guard, update process, or removal condition. That makes it easy for the local listener to drift from the reviewed upstream template or from the pinned reusable workflow contract.
    • Recommendation: Document how the mirrored listener should be updated, including how the pinned reusable-workflow SHA is selected and reviewed. Add a static check or snapshot policy that catches accidental drift in the local listener and the pinned `uses:` ref.
    • Evidence: The workflow says it `Mirrors templates/team-request-workflow.yml from NVIDIA/nvskills-ci@main`; the called reusable workflow is pinned to `d6c3af084edd278e5524c95e6586fc763835eb62`, but there is no sync/pinning regression check or update note in the changed files.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/request-nvskills-ci.yml:
- Line 32: The workflow currently references the reusable workflow as
NVIDIA/skills/.github/workflows/team-request.yml@main which is unpinned; replace
the `@main` ref with a specific commit SHA (e.g., @<commit-sha>) to pin the
reusable workflow and remove the supply-chain risk. Locate the uses line that
contains "NVIDIA/skills/.github/workflows/team-request.yml@main" and update it
to the commit SHA you obtain from the upstream repo (you can fetch the main
branch HEAD commit via the upstream repo's API or git), then commit the change
and optionally add Dependabot config to keep the pinned action updated.
🪄 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: 8898ecbc-910f-44b6-a4b2-b4611e6da99e

📥 Commits

Reviewing files that changed from the base of the PR and between 85580ae and 889c4ee.

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

Comment thread .github/workflows/request-nvskills-ci.yml Outdated
@jyaunches jyaunches merged commit 1e17f76 into main May 27, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.53 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants