Skip to content

fix(cli): cap sandbox name length validation#3639

Merged
cv merged 1 commit into
NVIDIA:mainfrom
1PoPTRoN:codex/validate-name-max-length
May 16, 2026
Merged

fix(cli): cap sandbox name length validation#3639
cv merged 1 commit into
NVIDIA:mainfrom
1PoPTRoN:codex/validate-name-max-length

Conversation

@1PoPTRoN

@1PoPTRoN 1PoPTRoN commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR makes the sandbox-name length limit explicit in the shared CLI validation path. Names that otherwise match the lowercase/hyphen format are now clearly constrained to 63 characters, so oversized values are rejected before they can reach OpenShell or runtime-facing paths.

Related Issue

Fixes #3638

Changes

  • Added a shared NAME_MAX_LENGTH constant set to 63 characters.
  • Updated validateName() to use the shared maximum instead of an inline value.
  • Updated validation guidance so users can see the length limit in prompts and error help.
  • Added a regression test for an extremely long, valid-looking sandbox name.
  • Kept related snapshot validation messaging aligned with the same 63-character rule.

Type of Change

  • 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

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Additional verification run locally:

  • npm run build:cli
  • cd nemoclaw && npm run build
  • npx vitest run test/runner.test.ts --project cli
  • npx vitest run nemoclaw/src/blueprint/snapshot.test.ts --project plugin
  • Targeted onboarding/deploy validation tests
  • npx biome check on changed files
  • git diff --check

Note: npm run typecheck:cli is currently blocked by unrelated missing @earendil-works/pi-coding-agent types in tools/e2e-advisor.

Summary by CodeRabbit

  • Bug Fixes

    • Name validation now enforces and clearly communicates a 1–63 character maximum for sandbox names.
  • Tests

    • Updated and added tests to verify the 1–63 character name constraint and the revised error messaging.

Review Change Stack


Signed-off-by: 1PoPTRoN vrxn.arp1traj@gmail.com

Copilot AI review requested due to automatic review settings May 16, 2026 01:18
@copy-pr-bot

copy-pr-bot Bot commented May 16, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR enforces a 63-character maximum for sandbox names via a new NAME_MAX_LENGTH constant, updates validation guidance and runner validation to use it, and adjusts tests to assert the 1-63 characters constraint.

Changes

Sandbox name length validation

Layer / File(s) Summary
Name validation constants and guidance
src/lib/name-validation.ts
NAME_MAX_LENGTH = 63 constant is exported and integrated into NAME_ALLOWED_FORMAT. The getNameValidationGuidance function is extended to append guidance text when name length exceeds the maximum.
CLI runner name validation
src/lib/runner.ts
validateName() imports NAME_MAX_LENGTH and enforces it in place of a hardcoded limit. The "too long" error message is updated to reference the configured maximum. Module exports are reordered without changing the set of exported symbols.
Test assertions for length constraint
src/lib/deploy/index.test.ts, test/e2e/brev-e2e.test.ts, test/onboard.test.ts, test/runner.test.ts
Deploy, E2E, and onboard test assertions are updated to expect the new 1-63 characters length constraint in validation error messages. A new test case in runner tests verifies that a 63-character name is accepted and an overlength name throws the specific "sandbox name too long (max 63 chars)" error.

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3599: Intersects on sandbox name validation and related oclif-compat help routing calling validateName.

Suggested reviewers

  • jyaunches

"I hopped through the code with careful paws,
Counted letters and tightened the laws,
Sixty-three steps is the limit I sing,
Tests now guard the small, proper string,
A rabbit's nod — validation in spring!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cli): cap sandbox name length validation' clearly summarizes the main change: adding a maximum length constraint to sandbox name validation.
Linked Issues check ✅ Passed The PR fully addresses issue #3638 by implementing the 63-character sandbox name limit, updating validation error messages, and adding regression tests.
Out of Scope Changes check ✅ Passed All changes are directly related to the sandbox name length validation objective; the export reordering in runner.ts is a minor, necessary refactoring tied to the constant import.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nemoclaw/src/blueprint/snapshot.ts Outdated
@1PoPTRoN 1PoPTRoN force-pushed the codex/validate-name-max-length branch from 65e08d3 to 7843b93 Compare May 16, 2026 01:29
Signed-off-by: 1PoPTRoN <vrxn.arp1traj@gmail.com>
@1PoPTRoN 1PoPTRoN force-pushed the codex/validate-name-max-length branch from 7843b93 to 07aecac Compare May 16, 2026 01:32
@cv

cv commented May 16, 2026

Copy link
Copy Markdown
Collaborator

@1PoPTRoN mind adding a DCO when you get a minute? Thanks!

@1PoPTRoN

Copy link
Copy Markdown
Contributor Author

@cv added DCO

@cv cv enabled auto-merge (squash) May 16, 2026 06:47
@cv cv disabled auto-merge May 16, 2026 06:48
@cv cv merged commit a8dff7a into NVIDIA:main May 16, 2026
21 of 24 checks passed
ericksoa pushed a commit that referenced this pull request May 18, 2026
## Summary
Updates the NemoClaw documentation for the v0.0.45 release by
summarizing the user-facing changes merged since v0.0.44 and bumping the
docs version metadata.
Refreshes generated user skills so agent-facing references match the
source docs.

## Changes
- Added v0.0.45 release notes covering onboarding recovery, local
inference, channel cleanup, share mount diagnostics, uninstall cleanup,
and security redaction updates.
- Updated command and troubleshooting docs for sandbox name limits, GPU
gateway reuse, DNS preflight behavior, channel removal cleanup, and
share mount path validation.
- Bumped docs version metadata to 0.0.45 and regenerated NemoClaw user
skills from the docs.
- Source summary: #3672 -> `docs/reference/commands.md`: documented
channel removal detaching bridge providers and un-applying channel
policy presets.
- Source summary: #3678 -> `docs/about/release-notes.md`: documented
Ollama streamed usage accounting in the release notes.
- Source summary: #3670 -> `docs/reference/commands.md`,
`docs/reference/troubleshooting.md`: documented safe GPU gateway
replacement behavior.
- Source summary: #3664 -> `docs/about/release-notes.md`: documented
blueprint permission normalization in the release notes.
- Source summary: #3181 -> `docs/reference/troubleshooting.md`:
documented GPU toolkit guidance when host drivers work but passthrough
is disabled.
- Source summary: #3554 -> `docs/about/release-notes.md`: documented
host `openshell-gateway` cleanup during uninstall.
- Source summary: #3651 -> `docs/reference/troubleshooting.md`:
documented the uncached `.invalid` DNS preflight probe.
- Source summary: #3643 -> `docs/reference/commands.md`: included
existing `NEMOCLAW_PROVIDER` interactive-mode behavior in generated
docs.
- Source summary: #3647 -> `docs/reference/commands.md`: documented
remote sandbox path verification for `share mount`.
- Source summary: #3646 -> `docs/reference/commands.md`: included
existing local writable mount target guidance in generated docs.
- Source summary: #3642 -> `docs/inference/use-local-inference.md`,
`docs/reference/commands.md`: documented managed-vLLM model override and
gated-model token checks.
- Source summary: #3639 -> `docs/reference/commands.md`: documented the
63-character sandbox name limit.

## Type of Change
- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Verification
- [ ] `npx prek run --all-files` passes
- [ ] `npm test` passes
- [ ] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [x] `make docs` builds without warnings (doc changes only)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

Commit hooks passed for the staged files. A standalone `npx prek run
--all-files` attempt was blocked by sandbox access to
`/Users/miyoungc/.cache/prek/prek.log`, so that checkbox is left
unchecked.

---
<!-- DCO sign-off required by CI. Run: git config user.name && git
config user.email -->
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Enhanced CLI command reference documentation with clearer guidance on
onboarding, GPU passthrough, inference configuration, channel removal,
and shared mounts.
* Improved troubleshooting sections with better DNS resolution and GPU
passthrough remediation steps.
  * Added documentation for overriding managed vLLM model selection.
* Updated release notes for v0.0.45 reflecting infrastructure and
workflow improvements.

* **Version Bump**
  * Released v0.0.45.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3755?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 -->
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI validateName should reject sandbox names over 63 characters

4 participants