fix(cli): correct SANDBOX_NAME precedence in start-services.sh

[BenediktSchackenberg]

Summary

Fixes #1309.

start-services.sh was using ${NEMOCLAW_SANDBOX:-${SANDBOX_NAME:-default}}, which means NEMOCLAW_SANDBOX always wins over a caller-set SANDBOX_NAME. When nemoclaw start passes SANDBOX_NAME=my-sandbox but the shell environment has a stale NEMOCLAW_SANDBOX=something-else (e.g. from a previous session or .env file), the PID directory and service routing uses the wrong sandbox name.

Fix

Reverse the precedence by resolving NEMOCLAW_SANDBOX into a local default first:

_SANDBOX_DEFAULT="${NEMOCLAW_SANDBOX:-default}"
SANDBOX_NAME="${SANDBOX_NAME:-$_SANDBOX_DEFAULT}"

Precedence is now:

1. --sandbox CLI flag (highest)
2. SANDBOX_NAME env var (caller-set)
3. NEMOCLAW_SANDBOX env var (session default)
4. "default" literal fallback

Testing

Verified the fix with:

export NEMOCLAW_SANDBOX=old-session
SANDBOX_NAME=my-sandbox ./scripts/start-services.sh --status
# → correctly uses my-sandbox, not old-session

Signed-off-by: Benedikt Schackenberg 6381261+BenediktSchackenberg@users.noreply.github.com

Summary by CodeRabbit

• Bug Fixes
  • Fixed sandbox environment variable precedence handling to ensure explicitly configured settings are respected and not inadvertently overridden by other environment variables.

[coderabbitai]

📝 Walkthrough

Walkthrough

The fix corrects environment variable precedence in start-services.sh so that an explicitly passed SANDBOX_NAME takes priority over an exported NEMOCLAW_SANDBOX, preventing PID directory mismatches during service operations.

Changes

Cohort / File(s)	Summary
Sandbox Name Precedence Fix scripts/start-services.sh	Reordered parameter expansion to prioritize explicit SANDBOX_NAME over NEMOCLAW_SANDBOX fallback, with "default" as ultimate fallback. Introduces intermediate _SANDBOX_DEFAULT variable to enforce correct precedence chain.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A sandbox name so oft confused,
Priority's now rightly used!
Explicit beats exported air,
No more PID files lost elsewhere! ✨

🚥 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 title directly and accurately summarizes the main change: correcting SANDBOX_NAME precedence in start-services.sh to fix the variable priority issue.
Linked Issues check	✅ Passed	The PR fully implements the fix requested in issue #1309 by reversing precedence order so SANDBOX_NAME takes priority over NEMOCLAW_SANDBOX, matching the expected behavior and suggested fix.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the SANDBOX_NAME precedence issue; no extraneous or unrelated modifications are present in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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.}

[Copilot]

Pull request overview

Fixes sandbox name resolution in start-services.sh so an explicitly caller-set SANDBOX_NAME is no longer overridden by a stale NEMOCLAW_SANDBOX environment variable, aligning PID directory/service routing with the intended sandbox selection.

Changes:

• Adjusted sandbox name defaulting logic to prefer SANDBOX_NAME over NEMOCLAW_SANDBOX.
• Added inline documentation explaining the intended precedence order and rationale.

---

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

[Copilot]

The SANDBOX_NAME precedence has changed here, but the existing test coverage appears to still assert the old behavior (e.g., test/service-env.test.js has a case expecting NEMOCLAW_SANDBOX to override SANDBOX_NAME, and those tests also don’t execute start-services.sh directly). Please update/add tests to assert the new precedence (SANDBOX_NAME wins over NEMOCLAW_SANDBOX) and ideally exercise the script logic rather than duplicating a parameter-expansion snippet in the test.

[coderabbitai]

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/start-services.sh`:
- Around line 22-26: Update the test expectation and the sibling script to
reflect the new SANDBOX_NAME precedence: change the test in
test/service-env.test.js for "uses NEMOCLAW_SANDBOX over SANDBOX_NAME" to expect
SANDBOX_NAME to win when both SANDBOX_NAME and NEMOCLAW_SANDBOX are set (i.e.,
expect "old" instead of "from-env"), and edit scripts/debug.sh to use the same
two-step expansion pattern as start-services.sh (create a fallback var like
_SANDBOX_DEFAULT="${NEMOCLAW_SANDBOX:-default}" and then
SANDBOX_NAME="${SANDBOX_NAME:-$_SANDBOX_DEFAULT}") so both scripts share
identical precedence logic.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1453b19c-e4c6-4b1f-9fcf-7f3126a29123

📥 Commits

Reviewing files that changed from the base of the PR and between db65da8 and 3f1adf6.

📒 Files selected for processing (1)

• scripts/start-services.sh

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify which scripts still use the old precedence pattern
echo "=== Scripts with old NEMOCLAW_SANDBOX:-SANDBOX_NAME pattern ==="
rg -n 'NEMOCLAW_SANDBOX:-.*SANDBOX_NAME' --type sh

echo ""
echo "=== Test files referencing the old pattern ==="
rg -n 'NEMOCLAW_SANDBOX:-.*SANDBOX_NAME' --type js --type ts

Repository: NVIDIA/NemoClaw

Length of output: 859

---

🏁 Script executed:

sed -n '120,160p' test/service-env.test.js

Repository: NVIDIA/NemoClaw

Length of output: 1646

---

🏁 Script executed:

sed -n '35,45p' scripts/debug.sh

Repository: NVIDIA/NemoClaw

Length of output: 310

---

Logic fix is correct; existing tests and debug.sh need updating to match new precedence.

The two-step expansion correctly implements the intended precedence (SANDBOX_NAME flag > SANDBOX_NAME env > NEMOCLAW_SANDBOX env > "default"). However, the following must be addressed:

1. Test failure: test/service-env.test.js test "uses NEMOCLAW_SANDBOX over SANDBOX_NAME" (around line 133) will fail. It expects NEMOCLAW_SANDBOX="from-env" to take precedence over SANDBOX_NAME="old", but the new pattern gives SANDBOX_NAME precedence, so the test will return "old" instead of the expected "from-env". Update the test expectations to reflect the new precedence.

2. Inconsistent sibling script: scripts/debug.sh:39 still uses the old pattern:

   SANDBOX_NAME="${NEMOCLAW_SANDBOX:-${SANDBOX_NAME:-}}"

   Update it to match the new precedence logic in start-services.sh.

🤖 Prompt for AI Agents

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

In `@scripts/start-services.sh` around lines 22 - 26, Update the test expectation
and the sibling script to reflect the new SANDBOX_NAME precedence: change the
test in test/service-env.test.js for "uses NEMOCLAW_SANDBOX over SANDBOX_NAME"
to expect SANDBOX_NAME to win when both SANDBOX_NAME and NEMOCLAW_SANDBOX are
set (i.e., expect "old" instead of "from-env"), and edit scripts/debug.sh to use
the same two-step expansion pattern as start-services.sh (create a fallback var
like _SANDBOX_DEFAULT="${NEMOCLAW_SANDBOX:-default}" and then
SANDBOX_NAME="${SANDBOX_NAME:-$_SANDBOX_DEFAULT}") so both scripts share
identical precedence logic.

[wscurran]

✨ Thanks for submitting this pull request, which proposes a way to fix sandbox name resolution in start-services.sh by correcting the precedence of environment variables.

---

Possibly related open issues:

• #1309 [NemoClaw][all platform] NEMOCLAW_SANDBOX env var overrides explicit SANDBOX_NAME in start-services.sh