fix(cli): resolve sandbox name for stop and status commands

[latenighthackathon]

Summary

nemoclaw stop and nemoclaw status call start-services.sh without passing SANDBOX_NAME, so the script defaults to "default". If the user named their sandbox anything else during onboarding (e.g. "research"), the PID directory lookup uses /tmp/nemoclaw-services-default/ instead of /tmp/nemoclaw-services-research/ — causing stop to miss running processes and status to always report services as stopped.

nemoclaw start already resolves the sandbox name from the registry via registry.listSandboxes().defaultSandbox. This applies the same resolution to stop() and showStatus() so all three commands use consistent PID directories.

Changes

• stop() now resolves defaultSandbox from the registry and passes it as SANDBOX_NAME (matching start())
• showStatus() reuses the defaultSandbox already fetched for the sandbox list display and passes it as SANDBOX_NAME

Root Cause

start-services.sh uses SANDBOX_NAME to construct the PID directory path (/tmp/nemoclaw-services-${SANDBOX_NAME}). When start() passes the correct name but stop() and status don't, they look in different directories and can't find the PID files.

Test Plan

• Verified start() already resolves sandbox name correctly (line 471-474)
• Confirmed stop() and showStatus() were missing the same resolution
• Full test suite: 650/653 passed (3 pre-existing failures in service-env.test.js)

Closes #1077

Summary by CodeRabbit

• Bug Fixes
  • Start, stop, and status commands now consistently apply the selected sandbox so service lifecycle operations and status checks run in the correct context.
  • Sandbox names provided to CLI commands are validated and safely quoted, preventing errors from unusual characters.

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

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Replaced leading SANDBOX_NAME= env-assignment with a new registrySandboxArg() that derives, validates, and shell-quotes a --sandbox from registry.listSandboxes().defaultSandbox; start(), stop(), and showStatus() now pass that computed --sandbox argument to the invoked scripts/commands.

Changes

Cohort / File(s)	Summary
CLI sandbox arg and callers bin/nemoclaw.js	Added registrySandboxArg(defaultSandbox) to obtain, validate (/^[a-zA-Z0-9._-]+$/) and shell-quote a --sandbox <name> suffix. Removed leading SANDBOX_NAME=... assignments; start() invokes start-services.sh with the computed --sandbox (after await ensureApiKey()), and stop()/showStatus() append the same --sandbox to their invocations so operations target the selected sandbox.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (bin/nemoclaw.js)
    participant Registry as Registry
    participant API as ensureApiKey
    participant Script as start-services.sh / stop/status scripts

    CLI->>Registry: listSandboxes() -> defaultSandbox
    Registry-->>CLI: default sandbox name
    CLI->>CLI: registrySandboxArg() validate & shell-quote "--sandbox <name>"
    CLI->>API: await ensureApiKey()
    API-->>CLI: API key ensured
    CLI->>Script: bash start-services.sh [--sandbox <name>] / stop/status with --sandbox
    Script-->>CLI: service PIDs / status (resolved per sandbox)

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰
I found a name beneath the hay,
Quoted it safe, then hopped away.
A tiny flag, a sandbox tune,
Services rise beneath the moon. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check	❓ Inconclusive	All changes are scoped to fixing sandbox name resolution in stop() and showStatus() functions. The removal of ensureApiKey from start() mentioned in commit history is not reflected in the provided raw_summary, making this assessment inconclusive.	Clarify whether ensureApiKey removal from start() is included in this PR or was handled separately, as it appears in commit messages but not in the file summary.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main fix: resolving sandbox name for stop and status commands, which is the core change shown in the summary.
Linked Issues check	✅ Passed	The PR successfully addresses issue #1077 by ensuring stop() and showStatus() consistently resolve the sandbox name from the registry and pass it to start-services.sh, matching start() behavior.

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

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 `@bin/nemoclaw.js`:
- Around line 478-481: The current code constructs an environment variable
SANDBOX_NAME (safeName/sandboxEnv) and invokes start-services.sh, but
scripts/start-services.sh prefers NEMOCLAW_SANDBOX and the registry-resolved
sandbox only wins when the script is called with --sandbox; change the
invocation in the run() calls (including the block using defaultSandbox /
safeName and the similar lines at 553-556 and in start()) to pass the resolved
sandbox via the --sandbox argument instead of SANDBOX_NAME, e.g. build a helper
that returns either `--sandbox <safeName>` or `""` and use that helper when
calling run(`bash "${SCRIPTS}/start-services.sh" --stop ...`) so the
registry-resolved sandbox consistently takes precedence; update references to
sandboxEnv, defaultSandbox, and run to use the new helper and remove the
SANDBOX_NAME env construction.

🪄 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: 9f91fc8b-f491-4db8-909c-38bd9725387b

📥 Commits

Reviewing files that changed from the base of the PR and between ce03233 and 5726235.

📒 Files selected for processing (1)

• bin/nemoclaw.js

[coderabbitai]

🧹 Nitpick comments (1)

bin/nemoclaw.js (1)

769-773: Optional: avoid re-reading registry in showStatus() to keep one snapshot.

Line 840 already resolves defaultSandbox, but Line 856 uses registrySandboxArg() which fetches registry again. Passing the already-read value keeps display/default and service-status lookup perfectly consistent within the same invocation.

Proposed refactor

-function registrySandboxArg() {
-  const { defaultSandbox } = registry.listSandboxes();
+function registrySandboxArg(defaultSandbox = registry.listSandboxes().defaultSandbox) {
   const safe = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
   return safe ? ` --sandbox ${shellQuote(safe)}` : "";
 }
@@
-  run(`bash "${SCRIPTS}/start-services.sh" --status${registrySandboxArg()}`);
+  run(`bash "${SCRIPTS}/start-services.sh" --status${registrySandboxArg(defaultSandbox)}`);

Also applies to: 838-841, 855-856

🤖 Prompt for AI Agents

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

In `@bin/nemoclaw.js` around lines 769 - 773, registrySandboxArg() re-reads the
registry snapshot causing inconsistency with the defaultSandbox resolved earlier
in showStatus(); change registrySandboxArg to accept an optional sandbox value
(e.g., registrySandboxArg(defaultSandbox)) and use that instead of calling
registry.listSandboxes() internally, then update callers (notably showStatus()
and the code around defaultSandbox resolution) to pass the already-read
defaultSandbox so the display and service-status use the same snapshot.

🤖 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 769-773: registrySandboxArg() re-reads the registry snapshot
causing inconsistency with the defaultSandbox resolved earlier in showStatus();
change registrySandboxArg to accept an optional sandbox value (e.g.,
registrySandboxArg(defaultSandbox)) and use that instead of calling
registry.listSandboxes() internally, then update callers (notably showStatus()
and the code around defaultSandbox resolution) to pass the already-read
defaultSandbox so the display and service-status use the same snapshot.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d9d23af8-d30a-4cc0-affe-d6f9ab08ba2f

📥 Commits

Reviewing files that changed from the base of the PR and between 498c25b and 93f4bb5.

📒 Files selected for processing (1)

• bin/nemoclaw.js

[latenighthackathon]

Closing — this is now resolved on main. The recent TypeScript refactor (#1306, #1307) replaced the shell-based start/stop/status with require('./lib/services') calls that properly pass sandboxName to startAll(), stopAll(), and showServiceStatus(). The original bug (sandbox name not forwarded to start-services.sh) no longer applies since the shell script path is no longer used.