fix(cli): clear sandbox registry when gateway is destroyed during onboard (fixes: #532)

[craigamcw]

Implemented feature with help from Claude Code

When onboard detects a stale gateway or starts a fresh one, it runs openshell gateway destroy which deletes all sandboxes in OpenShell. However the local NemoClaw registry (~/.nemoclaw/sandboxes.json) was not cleared, leaving stale entries that caused nemoclaw list to show sandboxes that no longer exist and nemoclaw <name> connect to fail with "sandbox not found".

Summary

Clear the local NemoClaw sandbox registry when a stale gateway is destroyed during onboard preflight. Previously, openshell gateway destroy deleted sandboxes in OpenShell but left stale entries in ~/.nemoclaw/sandboxes.json, causing nemoclaw list to show sandboxes that no longer exist.

Related Issue

#532

Changes

• Add clearAll() to registry module to reset all sandbox entries.
• Call registry.clearAll() after gateway destroy in both preflight cleanup and startGateway (covers all destroy paths).
• Add 3 tests for clearAll: multi-sandbox clear, disk persistence, and idempotent call on empty registry.

Type of Change

• Code change for a new feature, bug fix, or refactor.
• Code change with doc updates.
• Doc only. Prose changes without code sample modifications.
• Doc only. Includes code sample changes.

Testing

• make check passes.
• npm test passes.
• make docs builds without warnings. (for doc-only changes)

Checklist

General

• I have read and followed the contributing guide.
• I have read and followed the style guide. (for doc-only changes)

Code Changes

• make format applied (TypeScript and Python).
• Tests added or updated for new or changed behavior.
• No secrets, API keys, or credentials committed.
• Doc pages updated for any user-facing behavior changes (new commands, changed defaults, new features, bug fixes that contradict existing docs).

Doc Changes

• Follows the style guide. Try running the update-docs agent skill to draft changes while complying with the style guide. For example, prompt your agent with "/update-docs catch up the docs for the new changes I made in this PR."
• New pages include SPDX license header and frontmatter, if creating a new page.
• Cross-references and links verified.

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup: the local sandbox registry is now fully cleared when gateways are removed or replaced and during startup, preventing stale sandbox entries from persisting after restarts or removals.

• Tests

  • Added tests verifying registry reset behavior, that persistent storage is updated to an empty state, and that clearing is safe/idempotent when the registry is already empty.

[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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f6c41d52-50c1-4f08-abf1-9fc5bdfa81a8

📥 Commits

Reviewing files that changed from the base of the PR and between c819df6 and c8ceb6d.

📒 Files selected for processing (1)

• bin/lib/onboard.js

✅ Files skipped from review due to trivial changes (1)

• bin/lib/onboard.js

---

📝 Walkthrough

Walkthrough

Adds registry.clearAll() to wipe persisted sandbox registry and calls it from bin/lib/onboard.js when a prior nemoclaw gateway is destroyed; includes Vitest cases ensuring clearAll() clears in-memory state, updates the on-disk registry, and is idempotent.

Changes

Cohort / File(s)	Summary
Registry Module bin/lib/registry.js	Adds and exports clearAll() which acquires the registry lock and saves an empty state ({ sandboxes: {}, defaultSandbox: null }); adds JSDoc comments to existing functions.
Onboard Script bin/lib/onboard.js	Calls registry.clearAll() during stale/unnamed gateway cleanup in preflight() and when destroying a stale gateway in startGatewayWithOptions(); updates inline comments to document the registry-clearing behavior.
Tests test/registry.test.js	Adds three Vitest cases validating clearAll() removes all sandboxes and resets default, persists an empty sandboxes.json, and is safe/idempotent when registry already empty.

Sequence Diagram(s)

(Skipped — changes do not introduce a new multi-component control flow requiring visualization.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through sandboxes, one by one,
When gateways faded, my tidy job begun.
I nudged the registry, left defaults null,
Clean traces behind — calm, neat, and full.
Tiny paws, steady work, registry done.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding registry clearing when a gateway is destroyed during onboarding, directly addressing the issue.
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 unit tests (beta)

• Create PR with unit tests

---

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

[wscurran]

Thanks for submitting this proposed fix to clear the sandbox registry when a gateway is destroyed, which may help prevent issues with stale entries and improve the overall user experience.

[craigamcw]

Rebased onto current main — merges cleanly locally (fast-forward). GitHub's merge check is showing a conflict but git merge-tree confirms no actual conflicts in the changed files (bin/lib/onboard.js, bin/lib/registry.js, test/registry.test.js).

Happy to re-do anything if needed on your end.

[craigamcw]

@wscurran Thanks for the feedback! Just rebased onto current main — everything merges cleanly. When you get a chance, would you mind giving it a formal review?

[coderabbitai]

🧹 Nitpick comments (1)

bin/lib/registry.js (1)

240-243: Missing lock acquisition may cause race condition with concurrent registry operations.

The clearAll() function directly calls save() without using withLock(), unlike all other mutating functions in this module (registerSandbox, updateSandbox, removeSandbox, setDefault). If another process performs a read-modify-write operation concurrently, the clear could be silently lost.

In practice, clearAll() is only called during gateway destruction in onboarding, where concurrent modifications are unlikely. However, for consistency and defensive coding, wrapping the call in withLock() is safer.

♻️ Proposed fix to add lock

 function clearAll() {
-  save({ sandboxes: {}, defaultSandbox: null });
+  withLock(() => {
+    save({ sandboxes: {}, defaultSandbox: null });
+  });
 }

🤖 Prompt for AI Agents

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

In `@bin/lib/registry.js` around lines 240 - 243, The clearAll() function calls
save() directly and should acquire the same registry lock as other mutating
functions to avoid races; update clearAll to call withLock() and perform save({
sandboxes: {}, defaultSandbox: null }) inside the locked callback (mirroring
registerSandbox/updateSandbox/removeSandbox/setDefault patterns) so the registry
mutation is atomic under the same lock used by other operations.

🤖 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/lib/registry.js`:
- Around line 240-243: The clearAll() function calls save() directly and should
acquire the same registry lock as other mutating functions to avoid races;
update clearAll to call withLock() and perform save({ sandboxes: {},
defaultSandbox: null }) inside the locked callback (mirroring
registerSandbox/updateSandbox/removeSandbox/setDefault patterns) so the registry
mutation is atomic under the same lock used by other operations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7aa1efdf-2cab-4702-923e-a73d12c14e85

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc2102 and 5b21901.

📒 Files selected for processing (3)

• bin/lib/onboard.js
• bin/lib/registry.js
• test/registry.test.js

✅ Files skipped from review due to trivial changes (1)

• test/registry.test.js

[cv]

Closing in favor of #1245, which reimplements this fix on top of current main. The original approach here has gone stale — #1245 covers the same registry cleanup with CodeRabbit feedback addressed. Thanks @craigamcw for the original work.