Skip to content

fix(cli): align list default sandbox with env overrides (#1077) - #4866

Merged
cv merged 1 commit into
NVIDIA:mainfrom
Thabhelo:fix/list-env-default-and-test-hardening
Jun 5, 2026
Merged

fix(cli): align list default sandbox with env overrides (#1077)#4866
cv merged 1 commit into
NVIDIA:mainfrom
Thabhelo:fix/list-env-default-and-test-hardening

Conversation

@Thabhelo

@Thabhelo Thabhelo commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #4756 / #1077: nemoclaw list and inventory JSON now resolve the default sandbox from SANDBOX_NAME / NEMOCLAW_SANDBOX_NAME / NEMOCLAW_SANDBOX, matching status, start, and stop. Also consolidates #1077 test env isolation into shared beforeEach/afterEach hooks.

Related Issue

Related to #1077 (follow-up to merged #4756)

Changes

  • Wire resolveDefaultSandboxName() into getSandboxInventory() for defaultSandbox and isDefault rows
  • Add list/inventory tests for env-resolved default sandbox marker (*)
  • Refactor #1077 inventory tests into a nested describe with shared env save/restore
  • Add beforeEach env clearing in services.test.ts #1077 describe block

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 (targeted: src/lib/inventory/index.test.ts, src/lib/tunnel/services.test.ts, including -t '#1077')
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run 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)

Signed-off-by: Thabhelo 50872400+Thabhelo@users.noreply.github.com

Summary by CodeRabbit

  • Tests

    • Improved test organization and environment variable cleanup for better test reliability.
  • Refactor

    • Enhanced internal sandbox resolution logic for improved consistency.

Extend resolveDefaultSandboxName to getSandboxInventory so nemoclaw list
marks the env-resolved sandbox as default, matching status/start/stop.
Consolidate NVIDIA#1077 test env isolation into shared beforeEach/afterEach hooks.

Signed-off-by: Thabhelo <50872400+Thabhelo@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 5, 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 Jun 5, 2026

Copy link
Copy Markdown
Contributor

Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ced5aa94-a273-4108-ab54-460735364a3c

📥 Commits

Reviewing files that changed from the base of the PR and between 9b16a9e and 0e7f98a.

📒 Files selected for processing (3)
  • src/lib/inventory/index.test.ts
  • src/lib/inventory/index.ts
  • src/lib/tunnel/services.test.ts

📝 Walkthrough

Walkthrough

This PR implements environment-variable-driven default sandbox resolution in the inventory service by replacing direct default access with resolveDefaultSandboxName(), consolidates environment cleanup in test suites using shared lifecycle hooks, and adds comprehensive coverage for sandbox selection via SANDBOX_NAME and NEMOCLAW_SANDBOX_NAME environment variables.

Changes

Default Sandbox Resolution and Test Consolidation

Layer / File(s) Summary
Default sandbox resolution implementation
src/lib/inventory/index.ts
getSandboxInventory derives defaultSandbox via resolveDefaultSandboxName() instead of using recovery.defaultSandbox directly, and the inventory result and row construction now use this resolved value.
Inventory test environment lifecycle and coverage
src/lib/inventory/index.test.ts
Test imports add afterEach; a new describe("#1077 — env-resolved default sandbox") block captures, clears, and restores SANDBOX_NAME, NEMOCLAW_SANDBOX_NAME, and NEMOCLAW_SANDBOX in shared beforeEach/afterEach hooks. Tests cover sandbox resolution from SANDBOX_NAME env, JSON status resolution from NEMOCLAW_SANDBOX_NAME env, and list output formatting (marking the env-resolved sandbox with * and not marking the registry default).
Tunnel service test environment consolidation
src/lib/tunnel/services.test.ts
The #1077 test suite moves per-test environment variable cleanup into a shared beforeEach hook, removing redundant inline deletions of SANDBOX_NAME, NEMOCLAW_SANDBOX, and NEMOCLAW_SANDBOX_NAME from three individual test cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4756: Both PRs target sandbox env/default selection for #1077 status flows—this PR refactors env setup/teardown and adds resolution coverage while the related PR may introduce or refine the resolveDefaultSandboxName function.

Suggested labels

NemoClaw CLI, fix, area: cli, bug-fix, v0.0.60

Suggested reviewers

  • prekshivyas
  • cv

Poem

🐰 The sandbox chose its way,
Via env variables that sway,
Tests now share their cleanup chore,
No duplication anymore,
Resolution blooms—hooray, hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately captures the main change: aligning the list command's default sandbox resolution with environment variable overrides, which is the core objective of this fix.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

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

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

@Thabhelo

Thabhelo commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@prekshivyas — follow-up on your non-blocking review notes from #4756.

@cv cv added the v0.0.60 label Jun 5, 2026
@cv
cv merged commit 24fe907 into NVIDIA:main Jun 5, 2026
23 of 25 checks passed
miyoungc added a commit that referenced this pull request Jun 6, 2026
## Summary
- Adds the `v0.0.60` section to `docs/about/release-notes.mdx` using the
dev announcement from discussion #4877.
- Fills the source-doc gaps found during release-prep review across
inference, policy tiers, command behavior, security boundaries, Hermes
dashboard/tooling, runtime context, and troubleshooting.
- Refreshes generated agent skills under `.agents/skills/` from the
current Fern docs output and upgrades Fern from `5.44.3` to `5.45.0`.

## Source summary
- #4037 -> `docs/reference/architecture.mdx`,
`docs/about/how-it-works.mdx`, `docs/about/release-notes.mdx`: Documents
system-only runtime context that stays out of visible chat.
- #4875 -> `docs/reference/architecture.mdx`,
`docs/about/how-it-works.mdx`, `docs/about/release-notes.mdx`: Documents
try-first sandbox network/filesystem guidance and clearer failure
classification.
- #4788 -> `docs/security/best-practices.mdx`,
`docs/about/release-notes.mdx`: Documents shared OpenClaw
device-approval policy for startup and connect.
- #4768 -> `docs/reference/network-policies.mdx`,
`docs/network-policy/integration-policy-examples.mdx`,
`docs/get-started/quickstart.mdx`,
`docs/get-started/quickstart-hermes.mdx`, `docs/reference/commands.mdx`:
Documents `weather`, `public-reference`, and Hermes managed-tool gateway
preset behavior.
- #3788 and #4864 -> `docs/reference/network-policies.mdx`,
`docs/reference/commands.mdx`: Documents non-interactive policy-tier
fail-fast behavior and interactive prompt fallback.
- #4756 and #4866 -> `docs/reference/commands.mdx`: Documents env-aware
default sandbox resolution for `list`, `status`, and `tunnel` commands.
- #4320 -> `docs/reference/commands.mdx`: Documents `$$nemoclaw tunnel
status` behavior.
- #4328 -> `docs/reference/commands.mdx`: Documents line-scoped policy
preset descriptions in `policy-list`.
- #4580 and #4748 -> `docs/reference/architecture.mdx`: Documents
package-managed OpenShell gateway service and Docker-driver
gateway-marker behavior.
- #4598 -> `docs/manage-sandboxes/lifecycle.mdx`: Documents concurrent
gateway/dashboard cleanup isolation by sandbox name and port.
- #4777 -> `docs/reference/troubleshooting.mdx`: Documents Docker GPU
patch rollback behavior.
- #4610 -> `docs/reference/troubleshooting.mdx`,
`docs/reference/commands.mdx`: Keeps mutable OpenClaw config permission
guidance aligned and removes skipped experimental wording.
- #4868 -> `docs/reference/commands.mdx`: Keeps `.dockerignore` handling
for custom `onboard --from <Dockerfile>` contexts in generated skills.
- #4870 -> `docs/reference/commands.mdx`,
`docs/manage-sandboxes/runtime-controls.mdx`: Documents
`NEMOCLAW_MINIMAL_BOOTSTRAP` and generated skill coverage.
- #4641 -> `docs/inference/inference-options.mdx`,
`docs/reference/troubleshooting.mdx`: Documents local NVIDIA NIM
platform-digest pulls and served-model id adoption.
- #4810 and #4867 -> `docs/inference/inference-options.mdx`: Documents
stable NGC managed-vLLM image lineage and DGX Station DeepSeek V4 Flash
coverage.
- #4852 -> `docs/inference/use-local-inference.mdx`,
`docs/reference/troubleshooting.mdx`: Documents Ollama model fit
filtering, 16K context floor, cold-load retry, and failed-model
exclusion.
- #4847 -> `docs/inference/switch-inference-providers.mdx`: Documents
API-family sync, Hermes `api_mode`, and Bedrock Runtime exception.
- #4800 -> `docs/inference/tool-calling-reliability.mdx`: Documents
Nemotron managed-inference native tool-search fallback.
- #4333 -> `docs/inference/switch-inference-providers.mdx`: Documents
interactive multimodal input prompting.
- #4086 -> `docs/reference/troubleshooting.mdx`: Keeps proxy bypass
normalization in generated troubleshooting coverage.
- #4811 and #4855 -> `docs/get-started/quickstart-hermes.mdx`: Documents
prebuilt Hermes dashboard assets and TUI recovery without runtime
rebuilds.
- #4854 -> `docs/inference/switch-inference-providers.mdx`,
`docs/reference/commands.mdx`: Documents Hermes proxy API-key
placeholder preservation during inference switches.
- #4248 -> `docs/manage-sandboxes/messaging-channels.mdx`,
`.agents/skills/`: Keeps messaging enrollment behavior aligned with
manifest-hook implementation.
- #4771 -> `docs/security/best-practices.mdx`,
`docs/security/credential-storage.mdx`: Documents Hermes
placeholder-only secret boundary for sandbox-visible runtime files.
- #4787 -> `docs/security/best-practices.mdx`,
`docs/about/release-notes.mdx`: Documents expanded memory scanner
examples for OpenAI project keys and Slack app-level tokens.
- #4848 -> `docs/reference/commands.mdx`: Documents OpenClaw skill
install mirroring into the agent home directory.
- #4790 -> `docs/about/release-notes.mdx`: Uses the prior release-prep
structure and generated `.agents/skills/` refresh as the template for
this release.

## Verification
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix
nemoclaw-user --doc-platform fern-mdx`
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ skills/
--prefix nemoclaw-user --doc-platform fern-mdx --dry-run`
- `npm run docs`
- `git diff --check`
- skip-term scan across `docs/`, `.agents/skills/`, and `skills/`
- `npm run build:cli`
- `npm run typecheck:cli`
- Commit and pre-push hook suites, including markdownlint, gitleaks,
env-var docs gate, docs-to-skills verification, and skills YAML tests

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

## Summary by CodeRabbit

## Release Notes

* **New Features**
* DeepSeek-V4-Flash now available as default inference model for DGX
Station.
* Hermes dashboard improved with dedicated port and OAuth-authenticated
tool gateway selection.
* Added weather and public-reference policy presets for expanded agent
capabilities.
* Enhanced Ollama model selection with GPU memory filtering and
automatic retry for timeouts.

* **Bug Fixes**
  * Improved policy tier validation to prevent invalid configurations.
* Better sandbox cleanup scoping by port to prevent conflicts across
deployments.
  * Added GPU patch failure recovery with automatic rollback.

* **Documentation**
* Expanded troubleshooting guides for inference, security, and sandbox
lifecycle.
  * Added .dockerignore best practices for custom deployments.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
apurvvkumaria pushed a commit that referenced this pull request Aug 20, 2026
…#9534)

## Summary

`nemoclaw tunnel status` called `showStatus()` with no arguments, so it
resolved its PID directory from `NEMOCLAW_SANDBOX`, `SANDBOX_NAME` or
the literal string `default` instead of the sandbox that `tunnel start`,
`tunnel stop` and `nemoclaw status` target. On any host whose default
sandbox is not named `default` it read `/tmp/nemoclaw-services-default`
while the cloudflared PID file lived in
`/tmp/nemoclaw-services-<default-sandbox>`, and reported a running
tunnel as stopped. It also ignored the documented
`NEMOCLAW_SANDBOX_NAME` override. After this change the status command
performs the same `resolveDefaultSandboxName` step its siblings do.

## Related Issue

Fixes #9525

## Changes

- `src/commands/tunnel/status.ts` calls the existing
`resolveDefaultSandboxName()` authority directly and passes the resolved
name to `showStatus()`.
- `src/commands/simple-global-oclif-adapters.test.ts` extends the
existing case "maps tunnel and deprecated service commands to service
actions" to cover `TunnelStatusCommand`, asserting that `showStatus`
receives the resolved sandbox name rather than being called bare.

The first revision routed this through a new `runStatusCommand` /
`StatusCommandDeps` layer in `src/lib/tunnel/service-command.ts`,
mirroring `runStartCommand` and `runStopCommand`. That layer had a
single consumer, so it was removed in `0020c9874` in favour of calling
the resolver directly, and the two duplicate resolver cases it had added
to `service-command.test.ts` were removed with it; the existing resolver
tests already own environment priority and unsafe-name behaviour.
`src/lib/tunnel/service-command.ts` and `service-command.test.ts` are
therefore no longer part of this PR.

No new abstraction, configuration, fallback, or compatibility layer.
This restores the resolution step that `751459e74` (#4756) and
`24fe907f8` (#4866) established for `nemoclaw status` and `nemoclaw
list` on 2026-06-05, one day before `89cf90351` (#4320) added `tunnel
status` without it.

## Type of Change

- [x] 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)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

Both added tests fail on `main` and pass with this change. Without the
fix, the adapter case reports `runStatusCommand` "Number of calls: 0"
because the command still calls `showStatus` directly, and the two
`service-command.test.ts` cases fail with `TypeError: runStatusCommand
is not a function`.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run validate:pr` passed after refreshing `origin/main` when hooks
were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification:
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run 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)

Targeted tests:

```
npx vitest run --project cli src/lib/tunnel/service-command.test.ts \
  src/commands/simple-global-oclif-adapters.test.ts \
  src/lib/tunnel/services.test.ts src/lib/inventory/index.test.ts

Test Files  4 passed (4)
     Tests  108 passed (108)
```

Repository gates:

```
npm run validate:pr                     -> exit 0, TypeScript (CLI) Passed
npx prek run --from-ref <base> --to-ref HEAD
  -> Codebase growth guardrails Passed, Source-shape test budget Passed,
     Repository checks Passed, Oxfmt Passed, Oxlint fixes Passed, gitleaks Passed
npm --prefix nemoclaw run typecheck     -> clean
npx commitlint --from HEAD~1 --to HEAD  -> exit 0
```

The broad `npm test` gate is not checked: this change touches two small
CLI modules and their two owning test files, with no runtime or
test-harness change.

---

Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>


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

* **Bug Fixes**
* Tunnel status now consistently displays information for the resolved
default sandbox when no sandbox is explicitly selected.

* **Tests**
* Updated tunnel status coverage to validate default sandbox resolution
and the sandbox name passed to the status display.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
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.

3 participants