Conversation
Signed-off-by: oliver könig <okoenig@nvidia.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/sync-skills.yml (1)
27-29: Consider pinning to a commit SHA instead of@mainfor improved supply-chain security.The reusable workflow is referenced by branch (
@main), which means any change pushed to the template repository's main branch will automatically affect this workflow. Since aPATsecret is passed, a compromised or malicious update to the template could exfiltrate credentials.Pinning to a specific commit SHA (e.g.,
@63ee9e3b...) provides an auditable, immutable reference. You can still update periodically by bumping the SHA when template changes are reviewed.🔒 Example with SHA pinning
jobs: sync: - uses: NVIDIA-NeMo/FW-CI-templates/.github/workflows/_sync_skills.yml@main + uses: NVIDIA-NeMo/FW-CI-templates/.github/workflows/_sync_skills.yml@63ee9e3b9fc4ca02af1bd75d3126e526b2a77a24 secrets: PAT: ${{ secrets.PAT }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync-skills.yml around lines 27 - 29, The workflow currently pins the reusable workflow to a branch (`uses: NVIDIA-NeMo/FW-CI-templates/.github/workflows/_sync_skills.yml@main`), which is risky; update the `uses` reference to a specific commit SHA (e.g., replace `@main` with `@<commit-sha>`) so the reusable workflow is immutably pinned, then periodically bump that SHA after reviewing changes; ensure the `uses` line in the file remains the single place updated and keep the `secrets: PAT` mapping unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/sync-skills.yml:
- Around line 27-29: The workflow currently pins the reusable workflow to a
branch (`uses:
NVIDIA-NeMo/FW-CI-templates/.github/workflows/_sync_skills.yml@main`), which is
risky; update the `uses` reference to a specific commit SHA (e.g., replace
`@main` with `@<commit-sha>`) so the reusable workflow is immutably pinned, then
periodically bump that SHA after reviewing changes; ensure the `uses` line in
the file remains the single place updated and keep the `secrets: PAT` mapping
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 150eab27-2727-40e4-825e-82dfd2a69572
📒 Files selected for processing (1)
.github/workflows/sync-skills.yml
Summary
Replaces the self-contained
sync-skills.ymlworkflow with a thinworkflow_callwrapper that delegates to the shared reusable workflow inNVIDIA-NeMo/FW-CI-templates.Background
There is no common standard for markdown file discovery across agent frameworks — each tool looks in a different place. This PR addresses that gap by coupling the generic
AGENTS.md/skills/layout with a sync job that runs on push tomain, so every agent runtime finds the files it expects without per-framework duplication. The logic is centralised in FW-CI-templates so all repos pick up improvements automatically.What the delegated workflow does
NVIDIA-NeMo/FW-CI-templates/.github/workflows/_sync_skills.yml(called viaworkflow_call):PATsecretskills/→.claude/skillsand.agents/skillsAGENTS.md→CLAUDE.mdExample