ci(skills): align catalog refresh with repo gh-CLI PR pattern#4334
Conversation
The merged catalog-skills-refresh workflow pinned peter-evans/create-pull-request to a SHA that does not exist upstream, so every run fails at 'Prepare all required actions' with 'Unable to resolve action ... unable to find version'. Replace the third-party action with an inline gh-CLI script. This matches the pattern every other workflow in the repo uses for GitHub mutations (assign-linked-issue-author, pr-limit, e2e-advisor, pr-review-advisor) and removes the pin maintenance burden entirely. - Flip persist-credentials to true so 'git push' has a token. - Move the long PR body to .github/pr-bodies/catalog-skills-refresh.md to keep the workflow YAML readable. - Preserve existing labels (documentation, CI/CD) and the /nvskills-ci comment step. Refs #4282
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a PR body template and updates the catalog-skills-refresh workflow to preserve existing signer artifacts, replace the create-pr action with an inline gh-based commit/force-push + PR reuse/create flow, and change the signing request token to ChangesCatalog Skills Refresh Workflow Automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
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/pr-bodies/catalog-skills-refresh.md:
- Around line 4-9: Add a blank line immediately after the Markdown headings "##
Summary" and "## Validation" so they each have a separating empty line before
the following paragraph; update the section blocks that start with those
headings in the catalog-skills-refresh.md content to insert a single newline
after each heading to satisfy markdownlint rule MD022.
🪄 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: 21e4640e-af9a-4fd5-a721-fe32d861939a
📒 Files selected for processing (2)
.github/pr-bodies/catalog-skills-refresh.md.github/workflows/catalog-skills-refresh.yaml
PR Review AdvisorFindings: 0 needs attention, 4 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Add top-level heading and blank lines around headings/lists in .github/pr-bodies/catalog-skills-refresh.md to satisfy MD041, MD022, and MD032. Validated locally with markdownlint-cli2. Refs #4282
Address PR Review Advisor findings on the inline gh-CLI catalog refresh path: 1. Preserve NVSkills signer artifacts on branch reuse. Before regenerating the export, overlay skills/nemoclaw from the existing automation/catalog-skills-refresh branch (when it exists) into the working tree. The exporter already auto- preserves skill.oms.sig and skill-card.md from any existing export tree, so this overlay is what keeps signer artifacts from being dropped when the workflow force-pushes the refresh branch. 2. Narrow the write-token trust boundary. Restore persist-credentials: false on checkout so the GitHub token is not installed in git config while exporter code runs. Configure push credentials only inside the create/update step via 'gh auth setup-git', immediately before 'git push'. The diff check still works correctly after the overlay because 'git checkout origin/branch -- skills/nemoclaw' updates both the working tree and the index, so 'git diff' detects only the exporter's changes against the branch's prior export rather than reporting signer artifacts as spurious differences. Refs #4282
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/catalog-skills-refresh.yaml:
- Around line 64-66: The existing check uses `git ls-tree -d "origin/$BRANCH" --
skills/nemoclaw` which can succeed with empty output; replace or augment that
guard by verifying the path truly exists using `git cat-file -e
"origin/$BRANCH:skills/nemoclaw"` before running `git checkout "origin/$BRANCH"
-- skills/nemoclaw` so the `git checkout` is only executed when the overlay
object is present; update the conditional that currently references `git
ls-tree` to call `git cat-file -e` (or combine both checks) and ensure the
subsequent `rm -rf skills/nemoclaw` and `git checkout` remain inside the guarded
branch.
🪄 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: bae96fa8-9247-479b-a385-2c91a07ece4c
📒 Files selected for processing (1)
.github/workflows/catalog-skills-refresh.yaml
CodeRabbit and the PR Review Advisor both flagged that 'git ls-tree -d "origin/$BRANCH" -- skills/nemoclaw' can exit 0 with empty output when the tree exists but the path does not. In that case the next two lines would 'rm -rf skills/nemoclaw' and then fail at 'git checkout "origin/$BRANCH" -- skills/nemoclaw', aborting the workflow. 'git cat-file -e <ref>:<path>' fails when the object is absent, which is exactly the existence semantics we need. Refs #4282
…resh diff (#4342) <!-- markdownlint-disable MD041 --> ## Summary Two related fixes to the NemoClaw catalog skills export so the `Skills / Catalog Refresh` workflow actually opens a refresh PR on `main`: 1. **Detect untracked files in the change-detection diff.** The workflow ran `git diff --quiet` which only inspects tracked paths, so a freshly-exported catalog against an empty tracked tree looked unchanged and the workflow short-circuited via "Catalog skill export is already current." without ever pushing the 43 generated files. 2. **Flatten the export from `skills/nemoclaw/` to `skills/`.** Every other onboarded NVSkills product repo (`cuopt`, `nurec-skills`, `digital-health-skills`, `aiq`) — and `nvskills-ci` itself, with `skills/ci-smoke-test/SKILL.md` — uses `skills/<skill-name>/SKILL.md`. The per-product namespace layer in NemoClaw was redundant and didn't match anyone else's layout. ## Related Issue Follow-up to #4282 / #4284 / #4334. Observed on post-merge runs [26526695773](https://github.com/NVIDIA/NemoClaw/actions/runs/26526695773) and [26526772139](https://github.com/NVIDIA/NemoClaw/actions/runs/26526772139): both completed `success` after exporting 11 skills, then exited via `Stop after dry run` without producing a PR because no diff was detected against the empty `skills/` tree on `main`. ## Changes **Diff fix (commit 1):** - `.github/workflows/catalog-skills-refresh.yaml`: run `git add --intent-to-add` against the export paths before `git diff --quiet` so untracked files surface as additions. **Layout flattening (commit 2):** - `.agents/catalog-skills.yaml`: `export: skills/nemoclaw` → `export: skills` - `scripts/export-catalog-skills.py`: default target updated - `.github/workflows/catalog-skills-refresh.yaml`: every `skills/nemoclaw` reference (preserve-overlay, change-detection, commit-add) updated to `skills` - `.pre-commit-config.yaml`: hook regex now anchors to `skills/.*` - `.github/catalog-skills-signing-flow.md`, `.github/pr-bodies/catalog-skills-refresh.md`: prose updated - `test/catalog-skills-export.test.ts`: temp-fixture paths updated ## Why dropping the `nemoclaw/` subdir is safe Verified directly against `NVIDIA/nvskills-ci` (cloned locally): - `scripts/validate_request.py:21-26` — `WATCHED_PATH_PREFIXES = (".agents/skills/", "skills/", "team-skills/", "rules/team-rules/", "plugins/")` and `is_watched_path = path.startswith(WATCHED_PATH_PREFIXES)`. **Plain prefix match — no namespacing requirement.** - `.github/workflows/team-request.yml:114` mirrors the same `startswith("skills/")` filter. - `docs/team-onboarding.md:27` — *"Store NVSkills content under `skills/`, `team-skills/`, ..."* — no per-product subdir mentioned. - `nvskills-ci`'s own example skill is at `skills/ci-smoke-test/SKILL.md`. - Every other product repo follows the same pattern (verified via GitHub API on `cuopt`, `nurec-skills`, `digital-health-skills`, `aiq`). ## Type of Change - [x] 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 Reproduced and validated locally on a clean `main` checkout: ``` $ python3 scripts/export-catalog-skills.py Exported 11 catalog skill(s) to skills $ ls skills/ catalog-metadata.json nemoclaw-skills-guide nemoclaw-user-agent-skills nemoclaw-user-configure-inference ... (11 skills, flat layout) # Old change-detection (broken — misses untracked tree): $ git diff --quiet -- .agents/catalog-skills.yaml skills && echo changed=false || echo changed=true changed=false # Fixed change-detection: $ git add --intent-to-add -- .agents/catalog-skills.yaml skills $ git diff --quiet -- .agents/catalog-skills.yaml skills && echo changed=false || echo changed=true changed=true $ git diff --stat -- .agents/catalog-skills.yaml skills | tail -1 44 files changed, 8907 insertions(+), 1 deletion(-) ``` Vitest: ``` $ npx vitest run test/catalog-skills-export.test.ts --project cli ✓ |cli| test/catalog-skills-export.test.ts (4 tests) 1201ms Test Files 1 passed (1) Tests 4 passed (4) ``` - [x] `npx prek run --all-files` passes - [x] `npm test` passes for the affected test file (4/4) - [x] Tests updated for changed export path - [x] No secrets, API keys, or credentials committed - [ ] 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> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Refresh process updated to regenerate and preserve exports under the top-level skills/ directory, improving detection and staging of newly created files and signer artifacts. * Pre-commit checks widened to run against the broader skills/ export tree. * **Documentation** * Updated signing-flow and PR guidance to reference the skills/ export location. * **Tests** * Test fixtures aligned to the new skills/ export layout. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4342?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Summary
The Skills / Catalog Refresh workflow (merged via #4284 for issue #4282) fails immediately on every dispatch:
That SHA does not exist in the upstream
peter-evans/create-pull-requestrepo, so GitHub fails at "Prepare all required actions" before any job step runs. Failing run: 26518855615.Fix
Replace the third-party action with an inline
gh-CLI script. This is the pattern every other NemoClaw workflow uses for GitHub mutations:assign-linked-issue-author.yamlgh issue edit … --add-assignee,gh pr view,gh pr listpr-limit.yamlgh pr list,gh pr comment,gh pr closee2e-advisor.yaml/pr-review-advisor.yamlgh api,gh pr commentcatalog-skills-refresh.yaml(already, for/nvskills-ci)gh pr commentpeter-evans/create-pull-requestwas the only third-party mutation action in the repo, and the only place we had to maintain a SHA pin against an external action's release cadence.Changes
.github/workflows/catalog-skills-refresh.yamlpeter-evans/create-pull-request@<bad-sha>step.git checkout -B/git push --force-with-lease/gh pr createscript, idempotent against an existing open PR viagh pr list --head.persist-credentials: false→trueon the checkout sogit pushhas a token./nvskills-cistep fromsecrets.GITHUB_TOKENtogithub.tokenfor consistency with the rest of the workflow and the repo..github/pr-bodies/catalog-skills-refresh.md(new)Validation
actionlintclean (no new warnings).gh pr list --headinstead of creating a duplicate.if:condition as before.Notes
peter-evans/create-pull-requestto the realv7.0.8SHA (271a8d0340265f705b14b6d32b9829c1cb33d45e) instead. Choosing the inline script because it's the existing repo convention and removes the supply-chain pin entirely.Refs #4282
Summary by CodeRabbit
Chores
Documentation