fix: pass sandbox name to start-services.sh in stop and status

[phanisaimunipalli]

Summary

• nemoclaw stop and nemoclaw status always looked in /tmp/nemoclaw-services-default/ for PID files, regardless of the actual sandbox name.
• nemoclaw start correctly passes SANDBOX_NAME from the registry to start-services.sh, but stop() and showStatus() did not.
• When the registered default sandbox is named anything other than "default", nemoclaw stop reports the bridge as not running (and skips stopping it) and nemoclaw status shows it as stopped even though the bridge is alive.

Root cause

start-services.sh computes PIDDIR="/tmp/nemoclaw-services-${SANDBOX_NAME}". If SANDBOX_NAME is unset, it defaults to "default". stop() and showStatus() in nemoclaw.js did not set SANDBOX_NAME, so they always defaulted to /tmp/nemoclaw-services-default/, creating a mismatch with the directory start() had written the PID files to.

Fix

Apply the same registry-lookup and SANDBOX_NAME env prefix that start() already uses to both stop() and showStatus():

const { defaultSandbox } = registry.listSandboxes();
const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
const sandboxEnv = safeName ? `SANDBOX_NAME="${safeName}"` : "";
run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh" --stop`);

showStatus() already destructured defaultSandbox from registry.listSandboxes() for the sandbox list display, so the fix there is just two additional lines before the run() call.

Fixes #587.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Enhanced the stop and status commands to properly retrieve and validate the default sandbox configuration before execution, ensuring more reliable command behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

The stop() and showStatus() functions in bin/nemoclaw.js now retrieve the default sandbox name from the registry, validate it against a regex pattern, and conditionally pass it as a SANDBOX_NAME environment variable to the bash script for status and stop operations.

Changes

Cohort / File(s)	Summary
Sandbox Name Validation bin/nemoclaw.js	Modified stop() and showStatus() to retrieve defaultSandbox from registry.listSandboxes(), validate it against /^[a-zA-Z0-9._-]+$/, and conditionally set SANDBOX_NAME environment variable when invoking the bash script. When validation fails or sandbox is absent, the variable is omitted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

Hop, hop! The sandbox name is found,
Validated safe and sound,
Now status speaks the truthful way,
The bridge won't hide, come what may! 🐰

🚥 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 clearly and specifically describes the main change: passing sandbox name to start-services.sh in the stop and status commands.
Linked Issues check	✅ Passed	The PR correctly implements the required fix from issue #587 by applying registry lookup and safe-name validation logic to stop() and showStatus() functions to pass SANDBOX_NAME environment variable, ensuring all commands use the same PIDDIR.
Out of Scope Changes check	✅ Passed	All changes in bin/nemoclaw.js are directly related to fixing issue #587 by passing sandbox name to start-services.sh in stop and status commands; no out-of-scope modifications detected.

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

[coderabbitai]

🧹 Nitpick comments (1)

bin/nemoclaw.js (1)

246-248: Consider extracting sandbox env-prefix construction into a shared helper.

start(), stop(), and showStatus() now repeat the same safe-name/env assembly. Centralizing this reduces future drift risk.

Refactor sketch

+function getSandboxEnvPrefix() {
+  const { defaultSandbox } = registry.listSandboxes();
+  const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox)
+    ? defaultSandbox
+    : null;
+  return safeName ? `SANDBOX_NAME="${safeName}" ` : "";
+}
+
 async function start() {
   await ensureApiKey();
-  const { defaultSandbox } = registry.listSandboxes();
-  const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
-  const sandboxEnv = safeName ? `SANDBOX_NAME="${safeName}"` : "";
-  run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh"`);
+  run(`${getSandboxEnvPrefix()}bash "${SCRIPTS}/start-services.sh"`);
 }
 
 function stop() {
-  const { defaultSandbox } = registry.listSandboxes();
-  const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
-  const sandboxEnv = safeName ? `SANDBOX_NAME="${safeName}"` : "";
-  run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh" --stop`);
+  run(`${getSandboxEnvPrefix()}bash "${SCRIPTS}/start-services.sh" --stop`);
 }
 
 function showStatus() {
   // Show sandbox registry
   const { sandboxes, defaultSandbox } = registry.listSandboxes();
@@
-  const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
-  const sandboxEnv = safeName ? `SANDBOX_NAME="${safeName}"` : "";
-  run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh" --status`);
+  run(`${getSandboxEnvPrefix()}bash "${SCRIPTS}/start-services.sh" --status`);
 }

🤖 Prompt for AI Agents

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

In `@bin/nemoclaw.js` around lines 246 - 248, The sandbox env-prefix assembly
(safeName validation and SANDBOX_NAME="..." string) is duplicated in start(),
stop(), and showStatus(); extract this into a single helper (e.g.,
getSandboxEnvPrefix or buildSandboxEnvPrefix) that accepts defaultSandbox,
validates with /^[a-zA-Z0-9._-]+$/, and returns either the prefixed string
(SANDBOX_NAME="...") or an empty string; update start(), stop(), and
showStatus() to call that helper and use its return value in run(...) instead of
reimplementing safeName and sandboxEnv locally to avoid drift.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@bin/nemoclaw.js`:
- Around line 246-248: The sandbox env-prefix assembly (safeName validation and
SANDBOX_NAME="..." string) is duplicated in start(), stop(), and showStatus();
extract this into a single helper (e.g., getSandboxEnvPrefix or
buildSandboxEnvPrefix) that accepts defaultSandbox, validates with
/^[a-zA-Z0-9._-]+$/, and returns either the prefixed string (SANDBOX_NAME="...")
or an empty string; update start(), stop(), and showStatus() to call that helper
and use its return value in run(...) instead of reimplementing safeName and
sandboxEnv locally to avoid drift.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 425a89cb-dd99-41b9-8ac9-2350d6795b1f

📥 Commits

Reviewing files that changed from the base of the PR and between e52c2f0 and c069bcf.

📒 Files selected for processing (1)

• bin/nemoclaw.js