fix(onboard): persist model and provider per sandbox in registry (#1689)

[brandonpelfrey]

Summary

nemoclaw list and nemoclaw status show incorrect model/provider information for multi-sandbox setups because (1) setupInference() writes the registry update to the gateway name "nemoclaw" instead of the actual sandbox name, silently no-oping, and (2) registerSandbox() in createSandbox() never includes model or provider in the initial entry. This PR fixes both issues and also fixes a Bash 3.2 compatibility regression in uninstall.sh introduced by #1957.

Related Issue

Fixes #1689

Changes

• src/lib/onboard.ts (line ~5397): Pass sandboxName instead of GATEWAY_NAME to setupInference() so the internal registry.updateSandbox() call writes model/provider to the correct sandbox entry.
• src/lib/onboard.ts (line ~3005): Include model and provider in the registerSandbox() call inside createSandbox() so the initial registry entry is born with the correct inference config.
• uninstall.sh (line ~327): Replace local -A (Bash 4+ associative array) with a portable string-based dedup that works on macOS Bash 3.2, and guard empty-array iteration against set -u unbound variable errors.

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)

AI Disclosure

• AI-assisted — tool: Claude Code (pi agent)

---

Signed-off-by: Brandon Pelfrey bpelfrey@nvidia.com

Summary by CodeRabbit

• Improvements
  • Enhanced sandbox configuration tracking to capture model and provider information
  • Updated inference setup process for improved sandbox identification
  • Refined system process cleanup during uninstallation for better compatibility

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add model and provider field persistence to per-sandbox registry records during onboarding, modify the setupInference call to receive the sandbox name instead of a gateway constant, and refactor the deduplication logic in the uninstall script to use string matching instead of Bash associative arrays.

Changes

Cohort / File(s)	Summary
Sandbox Onboarding src/lib/onboard.ts	Records model and provider fields in sandbox registry during creation (set to model || null and provider || null). Changes setupInference(...) call argument from GATEWAY_NAME to sandboxName to apply inference setup per-sandbox rather than to a shared gateway identifier.
Process Cleanup Script uninstall.sh	Refactors stop_orphaned_openshell_processes() deduplication to replace Bash associative arrays with a string accumulator (_seen) and case substring matching. Adds conditional guard to skip deduplication loop when pids is empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A sandbox now remembers
Its model, its provider true,
No more confusion in the registry—
Each config gets its due!
And cleanup scripts run cleaner too. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title directly matches the main objective: persisting model and provider per sandbox in the registry, which is the primary fix for issue #1689.
Linked Issues check	✅ Passed	All coding objectives from issue #1689 are addressed: passing sandboxName to setupInference(), including model/provider in registerSandbox(), and making uninstall.sh macOS-compatible.
Out of Scope Changes check	✅ Passed	All changes directly support fixing issue #1689: src/lib/onboard.ts changes address registry updates, and uninstall.sh portability change resolves a macOS compatibility issue preventing proper cleanup.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/persist-sandbox-model-provider-1689

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/onboard.ts (1)

5395-5402: ⚠️ Potential issue | 🟡 Minor

Add explicit guard or clarify that setupInference's updateSandbox call is intentional before sandbox creation.

setupInference (line 3717) calls registry.updateSandbox(sandboxName, { model, provider }) at line 3865. On a fresh run, sandboxName is null here (initialized at line 5335, not assigned until createSandbox() at line 5474). However, updateSandbox() safely no-ops because its guard checks if (!data.sandboxes[name]) return false;, which catches the null/missing entry case. The pattern is already documented at lines 5487-5488 (referencing issue #1881), but this earlier call site lacks that context. Either add an explicit guard like if (sandboxName) registry.updateSandbox(...) before the setupInference() call, or add a clarifying comment explaining the intentional deferred update.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard.ts` around lines 5395 - 5402, The call to
setupInference(sandboxName, model, provider, endpointUrl, credentialEnv) may
cause registry.updateSandbox(sandboxName, { model, provider }) inside
setupInference (via the updateSandbox call in that function) before sandboxName
is set by createSandbox(); add an explicit guard or clarifying comment: either
wrap the setupInference invocation with a check if (sandboxName) to avoid
passing null to setupInference/registry.updateSandbox, or leave the call but add
a concise comment at the startRecordedStep/setupInference call site noting that
setupInference intentionally calls registry.updateSandbox and that updateSandbox
no-ops when the sandbox is not yet registered (referencing createSandbox and the
existing guard in updateSandbox) so reviewers know this is deliberate.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 5395-5402: The call to setupInference(sandboxName, model,
provider, endpointUrl, credentialEnv) may cause
registry.updateSandbox(sandboxName, { model, provider }) inside setupInference
(via the updateSandbox call in that function) before sandboxName is set by
createSandbox(); add an explicit guard or clarifying comment: either wrap the
setupInference invocation with a check if (sandboxName) to avoid passing null to
setupInference/registry.updateSandbox, or leave the call but add a concise
comment at the startRecordedStep/setupInference call site noting that
setupInference intentionally calls registry.updateSandbox and that updateSandbox
no-ops when the sandbox is not yet registered (referencing createSandbox and the
existing guard in updateSandbox) so reviewers know this is deliberate.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 19ab9882-d93b-487f-b174-722e0dce989b

📥 Commits

Reviewing files that changed from the base of the PR and between de9e409 and fe5d7a0.

📒 Files selected for processing (2)

• src/lib/onboard.ts
• uninstall.sh

[ericksoa]

LGTM. Three clean fixes:

1. setupInference() call: GATEWAY_NAME → sandboxName — the registry.updateSandbox() inside setupInference() (line 3959) was silently no-oping because no sandbox entry has the gateway name. Clear bug.

2. registerSandbox() in createSandbox(): Adding model and provider to the initial entry is the right call — these should be recorded at creation time, not deferred to a later updateSandbox.

3. uninstall.sh Bash 3.2 compat: The local -A → string-based dedup is portable and correct. The set -u guard on empty arrays is also needed.

Note: this will need a trivial merge with #1967 which also modifies the registerSandbox() call in createSandbox() (adds providerCredentialHashes). No semantic conflict — just adjacent lines.