fix(setup): use dynamic sandbox name instead of hardcoded 'nemoclaw'#284
Closed
kagura-agent wants to merge 3 commits intoNVIDIA:mainfrom
Closed
fix(setup): use dynamic sandbox name instead of hardcoded 'nemoclaw'#284kagura-agent wants to merge 3 commits intoNVIDIA:mainfrom
kagura-agent wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Replace all hardcoded 'nemoclaw' sandbox name references in setup.sh with a $SANDBOX_NAME variable that: 1. Accepts the name as the first CLI argument ($1) 2. Falls back to reading from ~/.nemoclaw/sandboxes.json (the onboard wizard's registry) to respect the name chosen during onboarding 3. Defaults to 'nemoclaw' if neither is available This fixes the split-brain issue where the onboard wizard stores a custom sandbox name in the registry but setup.sh always creates a sandbox named 'nemoclaw', causing all subsequent commands (connect, policy-add, Telegram bridge) to fail with 'sandbox not found'. Fixes NVIDIA#197
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe setup.sh script now resolves SANDBOX_NAME from an optional arg, ~/.nemoclaw/sandboxes.json, or a default; replaces hardcoded "nemoclaw" uses; adds gateway health checks, conditional CoreDNS patching for Colima, and provider preconfiguration using the dynamic name. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Setup as scripts/setup.sh
participant Config as "~/.nemoclaw/sandboxes.json"
participant Gateway
participant OpenShell
participant CoreDNS
participant Providers
User->>Setup: run setup.sh [optional SANDBOX_NAME]
Setup->>Config: read sandbox entry (if present)
alt arg provided
Setup->>Setup: SANDBOX_NAME = arg
else config present
Setup->>Setup: SANDBOX_NAME = config value
else
Setup->>Setup: SANDBOX_NAME = "nemoclaw"
end
Setup->>Gateway: start gateway
Gateway-->>Setup: started
loop health check
Setup->>Gateway: poll /health
Gateway-->>Setup: healthy? (yes/no)
end
alt Colima
Setup->>CoreDNS: apply CoreDNS patch
CoreDNS-->>Setup: patched
end
Setup->>Providers: configure providers using SANDBOX_NAME
Providers-->>Setup: providers ready
Setup->>OpenShell: sandbox create/get/delete using SANDBOX_NAME
OpenShell-->>Setup: sandbox status/details
Setup-->>User: final messages referencing SANDBOX_NAME
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup.sh`:
- Around line 203-216: The readiness check is brittle because the inline grep in
SANDBOX_LINE can cause the script to exit under set -euo pipefail when the
sandbox isn't found and also allows substring matches; fix by capturing the full
sanitized output of openshell sandbox list into a variable (use 2>&1 and strip
ANSI), avoid piping directly into grep so the pipeline can't fail (append "||
true" where appropriate), then locate the exact sandbox row using awk equality
on the name column (e.g., awk '$<col> == "'"$SANDBOX_NAME"'" {print $0}') to set
SANDBOX_LINE, then handle the case where SANDBOX_LINE is empty by emitting a
clear warn/fail before proceeding to compute SANDBOX_PHASE and SB_DETAIL; update
references to SANDBOX_LINE, SANDBOX_PHASE and SB_DETAIL accordingly so the later
image-pull checks and final fail message run only when a row is found.
- Around line 77-85: The registry-read section currently runs node inline and
silently falls back to "nemoclaw" on any failure; add an explicit Node.js
availability check (e.g., command -v node or which node) before attempting the
node -e invocation and emit a clear warning to stderr if Node is missing, then
proceed to a documented fallback; additionally capture the node command's exit
status and stderr when setting _sb_name and, on parse/runtime errors, print a
warning mentioning the registry path and error, then either retain the current
SANDBOX_NAME fallback or exit non‑zero per desired behavior. Update the block
that sets SANDBOX_NAME/_sb_name so it checks for node, warns on missing node,
runs node only when available, captures failures from the node -e call, and
prints those failures to stderr instead of silently defaulting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc0adf61-c986-4f0f-a65e-345506721ec7
📒 Files selected for processing (1)
scripts/setup.sh
Address review feedback: - Add explicit Node.js availability check before reading sandboxes.json - Warn (don't silently fallback) when registry lookup fails - Use awk exact-match instead of grep substring to prevent sandbox name collisions and set -euo pipefail abort on no match - Add explicit error when sandbox not found in list
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup.sh`:
- Around line 214-221: The current pipeline assigns SANDBOX_LINE by running
"openshell sandbox list" with "|| true", which hides command failures and treats
them the same as "no sandbox found"; instead run the raw command first capturing
both its stdout into a temp variable (e.g., raw_list) and its exit status, then
if the exit status is non-zero log/fail with the command error (including
stderr) before attempting to parse; only after a successful list (exit code 0)
run the sed/awk filtering to set SANDBOX_LINE and then fail with the existing
"Sandbox ... not found" message if that filtered result is empty. Reference the
SANDBOX_LINE assignment, the "openshell sandbox list" invocation, the "|| true"
suppression, and the final fail call so you can locate and replace the logic
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31574961-f304-4a19-a9c9-441f61e03b25
📒 Files selected for processing (1)
scripts/setup.sh
Address CodeRabbit review: the previous '|| true' on the openshell sandbox list pipeline collapsed command errors (CLI/gateway/auth issues) into the same empty-result path as 'sandbox not found'. Now we capture the command exit code separately and fail with the actual error message when the list command itself fails.
Contributor
Author
|
Addressed the remaining CodeRabbit review feedback:
Both issues were caught and fixed before these reviews landed. Thanks CodeRabbit! 🤖 |
wscurran
added
the
NemoClaw CLI
Contributor
Author
mafueee
pushed a commit
to mafueee/NemoClaw
that referenced
this pull request
Mar 28, 2026
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
2 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
Replace all hardcoded
nemoclawsandbox name references insetup.shwith a$SANDBOX_NAMEvariable that resolves dynamically.Problem
The onboard wizard (
bin/lib/onboard.js) prompts for a custom sandbox name and stores it in~/.nemoclaw/sandboxes.json. However,scripts/setup.shhardcodes--name nemoclaweverywhere, creating a split-brain situation:nemoclawFix
The
SANDBOX_NAMEvariable now resolves in this order:./scripts/setup.sh my-sandbox)~/.nemoclaw/sandboxes.json(the onboard wizard's registry)nemoclawAll 6 hardcoded references to the sandbox name in setup.sh have been replaced.
Testing
./scripts/setup.shwithout arguments still creates anemoclawsandbox./scripts/setup.sh my-botcreates a sandbox namedmy-botmy-assistantduringnemoclaw onboard, running./scripts/setup.shnow reads that name from the registryFixes #197
Summary by CodeRabbit
New Features
Chores