Skip to content

fix(install): read sandbox name from onboard session instead of stale registry#1846

Merged
ericksoa merged 1 commit intoNVIDIA:mainfrom
latenighthackathon:fix/install-stale-sandbox-name
Apr 15, 2026
Merged

fix(install): read sandbox name from onboard session instead of stale registry#1846
ericksoa merged 1 commit intoNVIDIA:mainfrom
latenighthackathon:fix/install-stale-sandbox-name

Conversation

@latenighthackathon
Copy link
Copy Markdown
Contributor

@latenighthackathon latenighthackathon commented Apr 13, 2026

Summary

resolve_default_sandbox_name() reads from ~/.nemoclaw/sandboxes.json,
which may hold a stale defaultSandbox entry from a previous gateway.
After re-onboarding (e.g. colima delete + reinstall), the installer
completion message shows the old sandbox name instead of the one just created.

Problem

The "Next:" completion message after curl|bash install always reads the
default sandbox name from the local registry. When the gateway has been
rebuilt, the old entry becomes stale but is still referenced:

Next:
  $ nemoclaw old-sandbox connect     ← wrong: stale name from sandboxes.json

Running nemoclaw old-sandbox connect then fails with
"Sandbox 'old-sandbox' is not present in the live OpenShell gateway."

Fix

Check ~/.nemoclaw/onboard-session.json first — it always contains the
sandbox name from the current onboard run. Fall back to the registry
only when no session file exists. Priority order:

  1. NEMOCLAW_SANDBOX_NAME env var (existing behavior, unchanged)
  2. onboard-session.jsonsandboxName (new: session-first)
  3. sandboxes.jsondefaultSandbox (existing fallback)
  4. "my-assistant" (existing default)

Test plan

  • Session has new sandbox + registry has stale → returns session name
  • No session file → falls back to registry name
  • NEMOCLAW_SANDBOX_NAME env var → overrides everything
  • Session exists but sandboxName is empty → falls through to registry
  • No session, no registry → returns "my-assistant" default
  • npm run build:cli clean
  • npm run typecheck:cli clean
  • test/install-preflight.test.ts — 56/56 pass

Closes #1839

Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

Summary by CodeRabbit

  • Chores
    • Improved installation process with enhanced sandbox configuration detection. The system now respects user preferences set via environment variables and remembers configuration from previous sessions. Fallback mechanisms have been refined to ensure reliable configuration discovery across various installation scenarios, providing users a more flexible and smoother overall setup experience with better backward compatibility.

… registry

resolve_default_sandbox_name() reads from sandboxes.json which may
contain a stale defaultSandbox entry from a previous gateway. After
re-onboarding (e.g. colima delete + reinstall), the completion message
shows the old sandbox name instead of the one just created.

Check the onboard session first — it always has the sandbox name from
the current run. Fall back to the registry only when no session exists.

Closes NVIDIA#1839

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d807e729-d55a-44d8-b6db-21deefd7cd9a

📥 Commits

Reviewing files that changed from the base of the PR and between a064e97 and 5dff7ef.

📒 Files selected for processing (1)
  • scripts/install.sh

📝 Walkthrough

Walkthrough

Updated the resolve_default_sandbox_name() function in the installation script to prioritize sandbox names from the current session (~/.nemoclaw/onboard-session.json) over stale entries in the local registry, with fallback to the previous behavior if the session file is unavailable.

Changes

Cohort / File(s) Summary
Session-based sandbox name resolution
scripts/install.sh
Modified resolve_default_sandbox_name() to check environment variable NEMOCLAW_SANDBOX_NAME, then attempt to read sandboxName from ~/.nemoclaw/onboard-session.json before falling back to ~/.nemoclaw/sandboxes.json. Ensures newly created sandboxes in the current session are referenced in completion messages.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A session remembered, no longer lost,
Fresh sandbox names in completion's cost,
Old stale entries fade to the back,
The script now knows just what it lacked! ✨

🚥 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 title accurately describes the main change: updating the installer to read sandbox name from the onboard session file instead of the stale local registry, which directly addresses the bug.
Linked Issues check ✅ Passed The code changes fully implement the requirements from issue #1839 by establishing the correct priority order for reading sandbox names: session file first, then registry, then default.
Out of Scope Changes check ✅ Passed All changes are scoped to the resolve_default_sandbox_name() function in scripts/install.sh and directly address the linked issue with no unrelated modifications.

✏️ 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.

@wscurran wscurran added good first issue Good for newcomers NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). fix labels Apr 14, 2026
@wscurran
Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this PR, which proposes a way to fix an issue with the completion message showing a stale default sandbox name.


Possibly related open issues:

@wscurran wscurran removed the good first issue Good for newcomers label Apr 14, 2026
@cv cv added the v0.0.16 Release target label Apr 14, 2026
Copy link
Copy Markdown
Contributor

@ericksoa ericksoa left a comment

Choose a reason for hiding this comment

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

LGTM — clean fix for stale sandbox name in installer completion message. Session-first priority is correct since install.sh triggers the onboard that writes it. Follows existing patterns, graceful fallback.

@ericksoa ericksoa merged commit 5b7f96a into NVIDIA:main Apr 15, 2026
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). v0.0.16 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms]completion message shows stale default sandbox name instead of newly created sandbox

4 participants