fix: align NeMo skill catalog install contracts - #1118
Open
mckornfield wants to merge 4 commits into
Open
Conversation
NVBugs: 6556546, 6556561, 6556564, 6556565 Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
mckornfield
force-pushed
the
6556546-skills-catalog-contracts/mck
branch
from
August 5, 2026 23:10
c23a938 to
ab09cef
Compare
mckornfield
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 ignored due to path filters (32)
📒 Files selected for processing (32)
🚧 Files skipped from review as they are similar to previous changes (27)
📝 WalkthroughWalkthroughSkill metadata now supports validated names and preconditions. Installers avoid duplicate ChangesSkill metadata and installation
Inference skill guidance
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/nemo_platform_ext/tests/cli/commands/skills/test_cli.py (1)
10-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep the expected path independent of
installed_skill_name.This test now uses the same helper as the installer. If that helper regresses, the production path and expected path can change together. Use an explicit or independently computed normalization expectation.
Proposed test adjustment
-from nemo_platform_ext.cli.commands.skills.base import installed_skill_name ... - installed_name = installed_skill_name(skill_name) + installed_name = ( + skill_name if skill_name.startswith("nemo-") else f"nemo-{skill_name}" + )Also applies to: 222-223
🤖 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 `@packages/nemo_platform_ext/tests/cli/commands/skills/test_cli.py` at line 10, Update the test expectations around the skill installation flow, including the cases near the imported installed_skill_name usage, so they no longer derive expected paths from installed_skill_name. Compute the expected normalized skill name independently or use an explicit expected value, while preserving the production helper usage in the code under test.
🤖 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
`@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/base.py`:
- Around line 14-18: Validate skill names before installation so path-like
values such as names containing separators or traversal components cannot escape
the intended install directory. Update installed_skill_name and the
BaseAgentInstaller.install flow to require a single safe path component, or
resolve the destination and enforce containment within the install root before
creating or writing files.
- Line 28: Update the Skill dataclass field ordering so preconditions follows
source_dist, or make preconditions keyword-only, preserving positional
Skill(..., raw, source_dir) calls and ensuring source_dir receives the path
while preconditions retains its default.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md`:
- Around line 561-563: Update the cleanup example to track the exact VM names or
IDs returned by each create command and delete only those tracked resources,
rather than listing and filtering all workspace VMs. Add an explicit
confirmation step before executing the deletions.
In
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.md`:
- Around line 25-26: Make the skill preconditions path-aware: in
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.md:25-26,
remove nemo_cli_available from the first-time bootstrap route; in
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.md:11-15,
scope provider_registered to VirtualModel chat guardrailing; and in
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-model-selection/SKILL.md:16-18,
scope provider_registered to Platform-routed model selection.
---
Nitpick comments:
In `@packages/nemo_platform_ext/tests/cli/commands/skills/test_cli.py`:
- Line 10: Update the test expectations around the skill installation flow,
including the cases near the imported installed_skill_name usage, so they no
longer derive expected paths from installed_skill_name. Compute the expected
normalized skill name independently or use an explicit expected value, while
preserving the production helper usage in the code under test.
🪄 Autofix
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: 8300bb51-83be-4fcf-9128-b92d8be45036
⛔ Files ignored due to path filters (31)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/claude.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/codex.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/cursor.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/opencode.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/base.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/cli.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/core/formatters.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/inference/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-agent-config/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-build-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-evaluator/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-experiments-upload/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-explore/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-files/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-guardrails/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-intake/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-model-selection/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-secrets/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-skill-selection/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-status/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-teardown/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-try-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_claude.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_codex.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_cursor.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_opencode.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_base.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_cli.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_skill_content.pyis excluded by!sdk/**
📒 Files selected for processing (31)
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/claude.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/codex.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/cursor.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/opencode.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/base.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/cli.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/core/formatters.pypackages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-agent-config/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-experiments-upload/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-explore/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-files/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-intake/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-model-selection/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-secrets/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-status/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-teardown/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-try-agent/SKILL.mdpackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_claude.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_codex.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_cursor.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_opencode.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_base.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_cli.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_skill_content.py
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md (1)
561-563: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winOther (CWE-20): Improper Input Validation
Reachability: External
Restrict cleanup to VMs created by this skill.
grep vm-matches unrelated VirtualModels. The loop then deletes every match inmy-workspace. Track the exact names or IDs returned by the create commands, and delete only those resources. Require confirmation immediately before deletion.🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md` around lines 561 - 563, Replace the cleanup loop’s broad `grep vm-` discovery with tracking of the exact VirtualModel names or IDs returned by each create command, and delete only those tracked resources in the `nemo inference virtual-models delete` calls. Add an explicit confirmation step immediately before deletion, without changing cleanup of unrelated resources.Source: Linters/SAST tools
🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md`:
- Around line 15-19: Make the bootstrap preconditions path-aware: in
packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md lines
15-19, scope workspace_exists, provider_registered, and secrets_configured to
the post-setup route or split setup into a separate skill; in
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.md
lines 25-26, scope nemo_cli_available to only the CLI-dependent route so
first-time bootstrap remains selectable.
---
Duplicate comments:
In `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md`:
- Around line 561-563: Replace the cleanup loop’s broad `grep vm-` discovery
with tracking of the exact VirtualModel names or IDs returned by each create
command, and delete only those tracked resources in the `nemo inference
virtual-models delete` calls. Add an explicit confirmation step immediately
before deletion, without changing cleanup of unrelated resources.
🪄 Autofix
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: f8c4dd80-fab2-4e7f-a607-dfa852e0c967
⛔ Files ignored due to path filters (31)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/claude.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/codex.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/cursor.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/agents/opencode.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/base.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/cli.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/core/formatters.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/inference/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-agent-config/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-build-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-evaluator/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-experiments-upload/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-explore/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-files/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-guardrails/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-intake/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-model-selection/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-secrets/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-skill-selection/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-status/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-teardown/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-try-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_claude.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_codex.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_cursor.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/agents/test_opencode.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_base.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_cli.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_skill_content.pyis excluded by!sdk/**
📒 Files selected for processing (31)
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/claude.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/codex.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/cursor.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/opencode.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/base.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/cli.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/core/formatters.pypackages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-agent-config/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-experiments-upload/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-explore/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-files/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-intake/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-model-selection/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-secrets/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-status/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-teardown/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-try-agent/SKILL.mdpackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_claude.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_codex.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_cursor.pypackages/nemo_platform_ext/tests/cli/commands/skills/agents/test_opencode.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_base.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_cli.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_skill_content.py
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-secrets/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/registry.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/opencode.py
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-model-selection/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-files/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-try-agent/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-status/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-intake/SKILL.md
- packages/nemo_platform_ext/tests/cli/commands/skills/test_base.py
- packages/nemo_platform_ext/tests/cli/commands/skills/agents/test_cursor.py
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.md
- packages/nemo_platform_ext/tests/cli/commands/skills/test_cli.py
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-explore/SKILL.md
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/claude.py
- packages/nemo_platform_ext/tests/cli/commands/skills/test_skill_content.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/cursor.py
- packages/nemo_platform_ext/tests/cli/commands/skills/agents/test_opencode.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/agents/codex.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/cli.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/formatters.py
- packages/nemo_platform_ext/tests/cli/commands/skills/agents/test_claude.py
- packages/nemo_platform_ext/tests/cli/commands/skills/agents/test_codex.py
mckornfield
commented
Aug 6, 2026
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/installer.py`:
- Line 30: Update the installer flow around validate_skill_name to precompute
each raw skill name’s installed_skill_name destination and reject duplicate
normalized destinations before any file operation. Ensure “foo” and “nemo-foo”
cannot proceed together, and add a regression test covering both raw names.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.md`:
- Around line 567-569: Update the cleanup confirmation flow around the
confirmation prompt to name the VirtualModels, provider, secret, and workspace
being deleted. After reading confirmation, explicitly exit before any cleanup
when its value is not exactly DELETE, rather than relying on the standalone test
command.
🪄 Autofix
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: 34697234-acab-48e4-8da4-5458d9d646ac
⛔ Files ignored due to path filters (10)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/base.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/installer.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/skills/registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/core/formatters.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/inference/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-guardrails/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_base.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_installer.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/skills/test_skill_content.pyis excluded by!sdk/**
📒 Files selected for processing (10)
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/base.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/installer.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/core/formatters.pypackages/nemo_platform_ext/src/nemo_platform_ext/skills/inference/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.mdpackages/nemo_platform_ext/tests/cli/commands/skills/test_base.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_installer.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_registry.pypackages/nemo_platform_ext/tests/cli/commands/skills/test_skill_content.py
💤 Files with no reviewable changes (1)
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-guardrails/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/skills/registry.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/formatters.py
- packages/nemo_platform_ext/tests/cli/commands/skills/test_skill_content.py
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
…g-contracts/mck Signed-off-by: Matt Kornfield <mkornfield@nvidia.com> # Conflicts: # packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-teardown/SKILL.md # sdk/python/nemo-platform/src/nemo_platform/skills/nemo-teardown/SKILL.md
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nemo-*skills do not becomenemo-nemo-*.NVBugs
Validation
uv run --frozen pytest packages/nemo_platform_ext/tests/cli/commands/skills -quv run --frozen ruff check ...on touched source/testsuv run --frozen ty check ...on touched sourcemake vendor-nemo-platform-extnemo skills list -f json --source nemo-platformand install smokeSummary by CodeRabbit
New Features
nemo-prefixes.Bug Fixes
Documentation
Review follow-up
d0d07aa99(fix: address skill catalog review feedback).150ca34e6(fix: harden skill installer cleanup handling) for the latest CodeRabbit findings.150ca34e6: focused source skill tests21 passed; focused vendored skill tests21 passed; targeted Ruff and ty checks passed;git diff --checkpassed.uv run pre-commit run -awas attempted, but pre-commit failed while fetchinghttps://github.com/norwoodj/helm-docs/with HTTP 503 before hooks ran.Latest push
ddc35cae5: merged currentorigin/mainand resolved thenemo-teardown/SKILL.mdconflict by keeping the precondition metadata plus main ClickHouse teardown guidance.make vendor-nemo-platform-ext; source skills pytest124 passed; focused vendored non-plugin skill pytest51 passed; targeted Ruff passed; PR diff whitespace check passed; DCO audit passed.uv run pre-commit run -astill fails before hooks run because GitHub returns HTTP 503 fetchingnorwoodj/helm-docs;uv run --frozen pytest ...now attempts to buildnemo-fabric-runtimeand fails with local Cargo 1.75 lacking edition2024 support.