fix(cli): cap installed skill names and preserve legacy removal by ericksoa · Pull Request #13488 · NVIDIA/NemoClaw

ericksoa · 2026-10-19T16:35:05Z

Outcome

Skill installation rejects names longer than 64 characters with a clear error instead of reporting that the source changed. Skill removal still accepts previously installed longer names and retains the existing character and path checks.

Reason

The Agent Skills specification caps new skill names at 64 characters. Applying that limit to the shared validator also prevented removal of existing longer names.

This carries forward Eric Curtin's contribution from #12503, preserving his commits and authorship. The replacement uses an NVIDIA branch because the required reviewed SDK job rejects fork PRs.

Related issues

Fixes #12406
Refs #12503

Changes

Enforce the 64-character installation limit and report it in the frontmatter error.
Enable the optional allowLegacyLength flag only in the removal action and canonical-root removal builder. This keeps installation strict while preserving legacy removal.
Cover installation boundaries, native and canonical-root legacy removal, unsafe names, and preservation of a neighboring skill directory.
Document the installation limit and legacy removal behavior across the OpenClaw, Hermes, and Deep Agents command references.
Link the headless deployment procedure to the canonical skill installation requirements.

Verification

Focused skill suites: npx vitest run src/lib/skill-install.test.ts src/lib/actions/sandbox/skill-install.test.ts src/commands/sandbox/skill.test.ts --coverage=false — 58 passed, 5 Linux-only tests skipped on macOS. Before the removal fix, the three legacy removal action cases failed.
NODE_OPTIONS=--max-old-space-size=8192 npm run typecheck:cli — passed.
Oxlint on the five changed TypeScript files — passed.
npm run docs — passed with zero errors. All three generated command-reference variants include the removal guidance, and each headless guide links to its matching installation section.
Normal pre-commit and pre-push hooks — passed, including publication validation, repository and growth checks, secret scanning, and CLI/plugin/JavaScript compiler checks.
All nine published commits are GitHub Verified, including the seven commits preserved from fix(cli): cap skill names at 64 characters #12503.
Core CI on c63dbaa — passed. The retained Linux test report records all 63 tests in the three focused skill suites passing, including actual legacy-directory removal, sibling preservation, and unsafe-name rejection.
Nine Advisor specialist reports on c63dbaa — all clear. Each report and its commit identity were inspected.
Managed-agent activation on c63dbaa — AMD64, ARM64, and rootless Podman passed.
Inspected the diff for secrets, API keys, credentials, and unrelated changes; none found.

Review notes

The original PR has a human approval from cv; that approval does not transfer to this replacement. The replacement requires independent review on its published commit.

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

Documentation
- Clarified skill installation requirements, including required SKILL.md frontmatter and which files persist across rebuilds.
- Documented the 64-character limit for new skill names and the handling of longer names during removal.
Bug Fixes
- Invalid skill names—including empty names, . and .., and names with unsupported characters—are rejected.
- Skills with legacy names longer than 64 characters can still be removed without affecting neighboring directories.

Fixes #12406 Signed-off-by: Eric Curtin <eric.curtin@docker.com>

# Conflicts: # docs/reference/commands.mdx

Signed-off-by: Eric Curtin <eric.curtin@docker.com>

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

copy-pr-bot · 2026-10-19T16:35:10Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-10-19T16:36:30Z

🌿 Preview your docs: https://nvidia-preview-pr-13488.docs.buildwithfern.com/nemoclaw

coderabbitai · 2026-10-19T16:37:08Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0c048cc5-f587-42df-987c-1919add802f2

📥 Commits

Reviewing files that changed from the base of the PR and between b01bb7f and c63dbaa.

📒 Files selected for processing: 7

docs/deployment/deploy-to-headless-server.mdx
docs/reference/commands.mdx
src/lib/actions/sandbox/skill-install.test.ts
src/lib/actions/sandbox/skill-install.ts
src/lib/skill-install.test.ts
src/lib/skill-install.ts
src/lib/skill-name.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

Skill-name validation now limits new installations to 64 characters and rejects empty, . and .. names. Removal accepts longer legacy names. Tests and documentation cover these rules.

Changes: Skill Name Rules

Layer / File(s)	Summary
Name validation and installation `src/lib/skill-name.ts`, `src/lib/skill-install.ts`, `src/lib/skill-install.test.ts`, `src/lib/actions/sandbox/skill-install.test.ts`, `docs/reference/commands.mdx`, `docs/deployment/deploy-to-headless-server.mdx`	Validation rejects names longer than 64 characters by default. Installation errors and documentation state the limit and skill frontmatter requirements. Tests cover accepted and rejected name lengths.
Legacy name removal `src/lib/skill-install.ts`, `src/lib/skill-install.test.ts`, `src/lib/actions/sandbox/skill-install.ts`, `src/lib/actions/sandbox/skill-install.test.ts`, `docs/reference/commands.mdx`	Removal allows names longer than 64 characters. Tests cover native removal, canonical-root fallback, preservation of sibling directories, and rejection of invalid names. Documentation clarifies the removal rules.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: rsliter, laitingsheng

Merge Risk: ⚪ Minimal · up to c63db

No actionable merge-blocking issue remains in the reviewed changes.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main changes: limiting newly installed skill names and preserving removal of legacy names.
Linked Issues check		Issue `#12406` requires a clear validation error for skill names above the 64-character Agent Skills limit. The change adds the 64-character limit to `validateSkillName`, includes the limit in the front…
Out of Scope Changes check		The changed implementation and tests directly support issue `#12406` and the stated legacy-removal outcome. The command references and headless deployment documentation describe the implemented installa…

Full details: Linked Issues check

Explanation

Issue #12406 requires a clear validation error for skill names above the 64-character Agent Skills limit. The change adds the 64-character limit to validateSkillName, includes the limit in the frontmatter error, and rejects the name before staging or SDK execution. The tests cover the 64-character boundary and longer names. Removal uses allowLegacyLength: true, so existing longer names remain removable without weakening installation validation.

Full details: Out of Scope Changes check

Explanation

The changed implementation and tests directly support issue #12406 and the stated legacy-removal outcome. The command references and headless deployment documentation describe the implemented installation and removal rules. No unrelated code or documentation change is established by the available PR summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

CI annotation context is incomplete: annotation collection unavailable or limited

_{Comment @coderabbitai help to get the list of available commands.}

github-code-quality · 2026-10-19T16:37:27Z

Code coverage overview

Languages: TypeScript

TypeScript / code-coverage/plugin

main 63002cd	codex/pr-12503-green c63dbaa	+/-
96%	97%	+1%

View coverage of the 10 most impacted files.

File	main 63002cd	codex/pr-12503-green c63dbaa	+/-
`nemoclaw/src/onboard/config.ts`	98%	96%	-2%
`nemoclaw/src/index.ts`	94%	93%	-1%
`nemoclaw/src/bl...print/runner.ts`	95%	95%	0%
`nemoclaw/src/bl...ime-identity.ts`	97%	97%	0%
`nemoclaw/src/bl...t-management.ts`	100%	100%	0%
`nemoclaw/src/co.../config-show.ts`	100%	100%	0%
`nemoclaw/src/commands/slash.ts`	100%	100%	0%
`nemoclaw/src/ru...time-context.ts`	100%	100%	0%
`nemoclaw/src/sh...er-boundary.cts`	100%	100%	0%
`nemoclaw/src/on...native-route.ts`	0%	100%	+100%

TypeScript / code-coverage/cli

main 63002cd	codex/pr-12503-green c63dbaa	+/-
84%	86%	+2%

View coverage of the 10 most impacted files.

File	main 63002cd	codex/pr-12503-green c63dbaa	+/-
`src/lib/onboard.ts`	62%	50%	-12%
`src/lib/state/l...diness-lease.ts`	66%	83%	+17%
`src/lib/inferen...ollama/proxy.ts`	41%	61%	+20%
`src/lib/onboard.../application.ts`	55%	84%	+29%
`src/lib/domain/...y/dimensions.ts`	0%	71%	+71%
`src/lib/inferen...er/lifecycle.ts`	0%	72%	+72%
`src/lib/actions...ry/operation.ts`	0%	76%	+76%
`src/lib/actions...try/snapshot.ts`	0%	78%	+78%
`src/lib/inferen...ovider/index.ts`	0%	88%	+88%
`src/lib/onboard...ternal-image.ts`	0%	91%	+91%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.

_{Updated October 19, 2026 17:30 UTC}

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

github-actions · 2026-10-19T17:36:26Z

PR Review Advisor finished for commit c63dbaa. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

Request review only when Require no Advisor blockers is green.

All previous runs

ericksoa · 2026-10-19T18:00:12Z

@coderabbitai full review

coderabbitai · 2026-10-19T18:28:27Z

✅ Action performed

Full review finished.

ericcurtin and others added 8 commits September 29, 2026 23:32

fix(cli): cap skill names at 64 characters

75406b9

Fixes #12406 Signed-off-by: Eric Curtin <eric.curtin@docker.com>

chore: merge main

c5342be

chore: merge main

722dcee

# Conflicts: # docs/reference/commands.mdx

test(cli): cover long skill name through install

b0c0444

Signed-off-by: Eric Curtin <eric.curtin@docker.com>

Merge branch 'main' into fix/skill-name-length

31b149f

docs(cli): state all skill name rules

630798c

Signed-off-by: Eric Curtin <eric.curtin@docker.com>

Merge remote-tracking branch 'origin/main' into fix/skill-name-length

c5eafa7

fix(cli): preserve removal of legacy skill names

40eb14a

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

docs(cli): link headless skill name requirements

c63dbaa

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

ericksoa marked this pull request as ready for review October 19, 2026 17:55

Navigation Menu

Repository navigation

fix(cli): cap installed skill names and preserve legacy removal - #13488

fix(cli): cap installed skill names and preserve legacy removal#13488
ericksoa wants to merge 9 commits into
mainfrom
codex/pr-12503-green

ericksoa commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

copy-pr-bot Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

github-code-quality Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

ericksoa commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ericksoa commented Oct 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Outcome

Reason

Related issues

Changes

Verification

Review notes

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-code-quality Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code coverage overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

ericksoa commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericksoa commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

github-code-quality Bot commented Oct 19, 2026 •

edited

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading