feat(cli): add sandbox skill remove command#4563
Merged
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
|
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 (7)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a sandbox skill removal feature: CLI command + metadata, action orchestration (removeSandboxSkill), SSH remote helpers for probing/removal/verification, name validation and OpenClaw mirror path support, extensive tests, and reference docs. ChangesSkill Removal Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
cv
added
NemoClaw CLI
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4563.docs.buildwithfern.com/nemoclaw |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: Dispatch required scenario E2E:
Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas This is an automated advisory review. A human maintainer must make the final merge decision. |
![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
🤖 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 `@src/lib/actions/sandbox/skill-install.ts`:
- Around line 93-96: The code incorrectly treats the literal skillName "help" as
a help sentinel which prevents uninstalling a real skill named "help"; change
the sentinel check in the skill install/remove flow to only treat "-h" and
"--help" (remove "help" from the condition), keep validateSkillName() unchanged
so "help" remains a valid skill name, and ensure calls to
printSkillInstallUsage() still occur for "-h"/"--help" only (update any
condition referencing skillName and the variables skillName,
validateSkillName(), and printSkillInstallUsage()).
- Around line 133-171: The try block currently calls process.exit() on several
failure paths which can prevent the finally cleanup
(fs.unlinkSync(tmpSshConfig)) from running; change those early-exit points
inside the try (the checks around skillInstall.checkExisting(ctx, paths), the
"not installed" branch, and the verification failure after
skillInstall.verifyRemove) to throw clear Errors (or set a local status and
return after the finally) instead of calling process.exit(), then handle
process.exit or process.exitCode at the top level after the try/finally; update
references to tmpSshConfig, skillInstall.checkExisting,
skillInstall.removeSkill, skillInstall.verifyRemove, skillName and sandboxName
to locate and replace the exit behavior so the finally block always runs and
cleanup always happens.
In `@src/lib/cli/public-display-defaults.ts`:
- Around line 372-377: The display metadata for the command identifier
"sandbox:skill:remove" uses flags: "<name>" which causes the CLI usage to render
an ambiguous second positional; update the metadata for sandbox:skill:remove to
use flags: "<skill>" instead of "<name>" so the rendered usage reads correctly
(e.g., nemoclaw <name> skill remove <skill>) and matches command/docs wording.
In `@src/lib/skill-install.ts`:
- Around line 331-342: The checkExisting function currently only tests for
SKILL.md and can return false when a partial upload left the directory present
but missing SKILL.md; update checkExisting (or add a new removal-specific helper
used by removeSandboxSkill) to probe directory existence too: test for the
uploadDir (and mirrorDir when paths.isOpenClaw && paths.mirrorDir) using a
directory-existence check (e.g., test -d ...) or combine file OR directory
checks so removeSandboxSkill will treat a present directory (even without
SKILL.md) as existing and allow cleanup; ensure sshExec calls and the
EXISTS/ABSENT logic (in checkExisting) are updated consistently to reflect the
new checks.
🪄 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: ae7bd40e-1879-4d9a-8766-0712f6ea5398
📒 Files selected for processing (10)
docs/reference/commands.mdxsrc/commands/sandbox/skill.test.tssrc/commands/sandbox/skill.tssrc/commands/sandbox/skill/remove.tssrc/lib/actions/sandbox/skill-install.tssrc/lib/cli/command-registry.test.tssrc/lib/cli/public-argv-translation.test.tssrc/lib/cli/public-display-defaults.tssrc/lib/skill-install.test.tssrc/lib/skill-install.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26677863025
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26678222063
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26692983320
|
miyoungc
added a commit
that referenced
this pull request
Jun 1, 2026
## Summary - Adds the v0.0.56 release notes section with links to the deeper docs pages for installer, status, inference, messaging, policy, and lifecycle changes. - Updates source docs for the remaining release-prep gaps around `uv` in the PyPI preset, compact WhatsApp pairing guidance, and `nemoclaw inference set` command boundaries. - Refreshes generated `nemoclaw-user-*` skills and removes skipped experimental command terms from generated skill surfaces. ## Source summary - #4613 -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents that public installs and `nemoclaw update` follow the maintained `lkg` tag by default. - #4419 -> `docs/about/release-notes.mdx`: Notes that non-interactive Linux installs can reactivate Docker group membership and continue in one installer run when `sg docker` is available. - #4550 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures live sandbox agent-version probing for status, connect, and upgrade checks. - #4609 -> `docs/inference/use-local-inference.mdx`, `docs/about/release-notes.mdx`: Captures the GPU Docker-driver host-network local-inference reachability gate. - #4607 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents compact WhatsApp QR pairing guidance and gateway/session diagnostics. - #4582 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Reflects Slack credential validation before enabling the channel. - #4554 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Keeps Telegram allowlist alias guidance in the generated user skills and release notes. - #4563 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Includes the new `nemoclaw <name> skill remove <skill>` command in command docs and release notes. - #4566 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents the `nemoclaw inference set` redirect boundary when `--provider` or `--model` is missing. - #4323 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures per-sandbox status JSON support. - #4506 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures debug command sandbox-name validation and safer tarball writing. - #4569 -> `docs/network-policy/integration-policy-examples.mdx`, `docs/about/release-notes.mdx`: Documents that the `pypi` preset allows `/usr/local/bin/uv`. - #4579 -> `docs/network-policy/integration-policy-examples.mdx`, `docs/about/release-notes.mdx`: Captures observable Jira preset validation guidance. - #4229 -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents user-data preservation defaults for uninstall. - #4399 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures CPU-only sandbox intent preservation across rebuilds. - #4058 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures safer snapshot restore behavior around existing destinations. - #4155 and #4460 -> skipped by `docs/.docs-skip`: Removed skipped experimental command terms from source docs and generated skill evals instead of documenting those features. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run docs` (passes; Fern reports the pre-existing light-mode accent contrast warning) - `rg "permissive mode|shields down|shields up|shields status|config rotate-token|rotate-token" .agents/skills` (no matches) - `npm run build:cli` (run to refresh local CLI artifacts for the pre-push TypeScript hook) - Commit hooks passed, including `NEMOCLAW_* env-var documentation gate`, `Verify docs-to-skills output`, `markdownlint-cli2`, `gitleaks`, and `Test (skills YAML)`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded Model Router setup with YAML examples, flow diagrams, and credential handling; strengthened agent-config immutability and integrity guidance; messaging channels updated (Telegram aliases, WhatsApp pairing/diagnostics); CLI docs revised (GPU detection, inference set behavior, uninstall/rebuild preservation); overview rebranded to NemoClaw and added v0.0.56 release notes. * **New Features** * Added `nemoclaw <name> channels status` (messaging diagnostics, JSON); added `nemoclaw <name> skill remove`; Hermes no longer marked experimental; DGX Spark quickstart sandbox-name note. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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
Salvages the skill removal work from #1962 onto a maintainer-owned, signed-off branch so the DCO-blocked contribution can proceed.
Adds
nemoclaw <sandbox> skill remove <skill>for removing installed agent skills from a running sandbox.Related Issue
Supersedes #1962.
Changes
sandbox skill removeoclif command and public argv translation..and...Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Validation run:
npm run build:clinpm test -- src/lib/skill-install.test.ts src/commands/sandbox/skill.test.ts src/lib/cli/public-argv-translation.test.ts src/lib/cli/command-registry.test.tsnpm run typecheck:clinpm run docs:strictnpm run docs:strictcompleted with no errors (Fern reported existing upgrade warnings).Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Tests