fix(onboard): remove provider detach sweep before sandbox creation

[sandl99]

Outcome

Fresh onboarding no longer detaches nine providers before the sandbox exists. Removing the calls eliminates the reported sandbox not found warnings. Existing-sandbox recreate and destroy cleanup remain intact.

Reason

The fresh-create plan incorrectly reused deletion cleanup before creating a sandbox. Remove that call and its unused callback and missing-sandbox tolerance option.

Runtime validation also exposed the pinned SDK rejecting an initial restart Error before the CLI recovery grace could apply.

Changes

• Delete the pre-create provider-detach sweep and unused plumbing. The onboarding regression requires zero detach commands.
• Poll SDK readiness with identity checks within the existing deadline. Share the CLI's existing limit of 20 initial Error observations and its terminal-phase classifier. Focused tests protect recovery, identity replacement, terminal failure, and timeout.
• Make the existing connect-test fixture report Ready after a successful start. Preserve its assertions and timeouts.

Verification

Commit: fd7b2466257bc4907bf5ab73b63149c9a4786051. Canonical main 6a02aac7f0053978a20eaed73159386e0ad2aff9 was merged without conflicts.

• Fresh-onboarding regression: nine detach commands before the removal, zero afterward.
• SDK lifecycle and start-wait suites: 55 tests passed. Ten terminal-phase cases failed before the repair and pass afterward.
• Six existing shared-classifier caller suites: 88 tests passed.
• Full CI: passed, including all 12 CLI shards, coverage, static checks, types, build, installer, plugin, and dependency checks.
• CPU runtime proof and portable-profile E2E: passed.
• Managed-image qualification: passed, including all image build and direct-startup prerequisites and Docker and Podman activation.
• Normal commit and publication hooks passed. Every published commit is GitHub Verified. The diff contains no secrets, API keys, or credentials.

Review notes

Self-review covers the full NVIDIA/NemoClaw diff at the commit above, including sensitive src/lib/onboard/** paths. Provider cleanup and lifecycle polling retain identity checks.

All nine Advisor specialist reviews are clear, and the blocker gate passed. Reports and artifact identities were verified; comments, reviews, inline comments, and threads contain no unresolved finding. Human review remains pending. No CI waiver or merge approval is claimed.

---

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Sandbox startup now checks readiness through repeated status updates, allowing recovery from up to 20 consecutive initial error statuses before failing. Terminal status detection is case-insensitive.
  • Creating a sandbox no longer detaches existing providers. Missing-sandbox errors during provider detachment are now reported as failures rather than treated as successful cleanup.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: cd3982a2-b883-4b31-b8bb-1e2e52aed3ea

📥 Commits

Reviewing files that changed from the base of the PR and between 6a02aac and fd7b246.

📒 Files selected for processing (18)

• src/lib/actions/sandbox/connect.ts
• src/lib/actions/sandbox/gateway-state.ts
• src/lib/actions/sandbox/start-wait.test.ts
• src/lib/adapters/openshell/sandbox-lifecycle-sdk.test.ts
• src/lib/adapters/openshell/sandbox-lifecycle-sdk.ts
• src/lib/domain/lifecycle/sandbox-start-error-grace.ts
• src/lib/onboard/managed-workload/onboard-orchestration.test.ts
• src/lib/onboard/managed-workload/onboard-orchestration.ts
• src/lib/onboard/sandbox-create-intent-types.ts
• src/lib/onboard/sandbox-create-plan-materialization.ts
• src/lib/onboard/sandbox-create-plan.test.ts
• src/lib/onboard/sandbox-create/orchestration.test.ts
• src/lib/onboard/sandbox-create/orchestration.ts
• src/lib/onboard/sandbox-provider-cleanup.ts
• src/lib/state/gateway.ts
• test/onboarding/onboard-messaging.test.ts
• test/runtime/sandbox/sandbox-provider-cleanup.test.ts
• test/support/connect-flow-test-harness.ts

💤 Files with no reviewable changes (6)

• src/lib/onboard/sandbox-create-intent-types.ts
• src/lib/onboard/sandbox-create/orchestration.test.ts
• src/lib/onboard/managed-workload/onboard-orchestration.ts
• src/lib/onboard/sandbox-provider-cleanup.ts
• src/lib/onboard/managed-workload/onboard-orchestration.test.ts
• src/lib/onboard/sandbox-create-plan-materialization.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

Sandbox starts and stops now use repeated sandbox-state observations. Sandbox creation no longer runs pre-delete provider cleanup, and missing-sandbox command errors are retained as detach failures.

Changes

Sandbox readiness polling

Layer / File(s)	Summary
Initial Error grace and connect readiness src/lib/domain/lifecycle/sandbox-start-error-grace.ts, src/lib/actions/sandbox/connect.ts, src/lib/actions/sandbox/gateway-state.ts, src/lib/actions/sandbox/start-wait.test.ts	A shared checker allows up to 20 consecutive initial Error observations when enabled. Connect readiness uses the checker, and the tests cover 11 and 20 observations.
SDK start and stop phase observation src/lib/adapters/openshell/sandbox-lifecycle-sdk.ts, src/lib/adapters/openshell/sandbox-lifecycle-sdk.test.ts, src/lib/state/gateway.ts, test/support/connect-flow-test-harness.ts	The lifecycle adapter repeatedly fetches sandbox state and checks identity and phase. It uses separate start and stop polling callbacks; tests cover recovery, failure phases, query failures, and timeouts.

Sandbox creation provider cleanup

Layer / File(s)	Summary
Remove pre-delete cleanup from create planning src/lib/onboard/sandbox-create-intent-types.ts, src/lib/onboard/managed-workload/onboard-orchestration.ts, src/lib/onboard/sandbox-create-plan-materialization.ts, src/lib/onboard/sandbox-create-plan.test.ts, src/lib/onboard/managed-workload/onboard-orchestration.test.ts	The create-plan contract and materialization path no longer accept or invoke the pre-delete cleanup callback. Related tests remove the callback and its assertions.
Provider detach authority and missing-sandbox handling src/lib/onboard/sandbox-create/orchestration.ts, src/lib/onboard/sandbox-provider-cleanup.ts, src/lib/onboard/sandbox-create/orchestration.test.ts, test/runtime/sandbox/sandbox-provider-cleanup.test.ts, test/onboarding/onboard-messaging.test.ts	Cleanup requires identity revalidation. Missing-sandbox command errors are recorded as failures, and the onboarding test asserts that new-sandbox creation does not detach providers.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Lifecycle as Sandbox lifecycle
  participant SDK as sandbox.get
  participant Poll as waitForStartPoll
  Lifecycle->>SDK: Fetch sandbox phase and identity
  SDK-->>Lifecycle: Return sandbox state
  Lifecycle->>Poll: Wait before the next start observation
  Poll-->>Lifecycle: Complete poll wait
  Lifecycle->>SDK: Fetch sandbox state again

Loading

Suggested reviewers: ericksoa, prekshivyas

Merge Risk: ⚪ Minimal · up to fd7b2

The change is mergeable after normal checks; the remaining test assertion protects the intended SDK migration.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 12 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: removing the provider detach sweep before sandbox creation.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit fd7b246 in the fix/onboard-missing-... branch is 97%. The line coverage in commit 63002cd in the main branch is 96%.

Show a line coverage summary of the most impacted files.

File	main 63002cd	fix/onboard-missing-... fd7b246	+/-
nemoclaw/src/onboard/config.ts	98%	96%	-2%
nemoclaw/src/index.ts	94%	93%	-1%
nemoclaw/src/bl...print/runner.ts	95%	95%	0%
nemoclaw/src/bl...ime-identity.ts	97%	97%	0%
nemoclaw/src/bl...t-management.ts	100%	100%	0%
nemoclaw/src/co.../config-show.ts	100%	100%	0%
nemoclaw/src/commands/slash.ts	100%	100%	0%
nemoclaw/src/on...native-route.ts	0%	100%	+100%

TypeScript / code-coverage/cli

The overall line coverage in commit fd7b246 in the fix/onboard-missing-... branch is 86%. The line coverage in commit 63002cd in the main branch is 84%.

Show a line coverage summary of the most impacted files.

File	main 63002cd	fix/onboard-missing-... fd7b246	+/-
src/lib/onboard.ts	62%	46%	-16%
src/lib/onboard...rchestration.ts	41%	32%	-9%
src/lib/state/sandbox.ts	92%	84%	-8%
src/lib/actions...ess-recovery.ts	65%	78%	+13%
src/lib/inferen...file/cleanup.ts	73%	88%	+15%
src/lib/state/l...diness-lease.ts	66%	82%	+16%
src/lib/inferen...ollama/proxy.ts	41%	61%	+20%
src/lib/onboard.../application.ts	55%	84%	+29%
src/lib/inferen...nvidia/index.ts	0%	85%	+85%
src/lib/onboard...ternal-image.ts	0%	91%	+91%

---

_{Updated October 09, 2026 04:10 UTC}

[github-actions]

PR Review Advisor finished for commit fd7b246. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

Request review only when Require no Advisor blockers is green.

All previous runs

[hunglp6d]

Review of fd7b2466257bc4907bf5ab73b63149c9a4786051 against main at 6a02aac7f0053978a20eaed73159386e0ad2aff9; both SHAs were rechecked and remain unchanged. I found no merge-blocking correctness or security issue in the current diff and recommend approval.

• Behavior and regression coverage: Fresh creation now removes the unnecessary detach sweep rather than suppressing its errors. Recreate retains provider cleanup before deletion, with identity revalidation. SDK start submits one mutation, verifies identity on every readiness observation, and permits at most 20 initial Error observations within the existing deadline. Another phase ends that allowance. Tests cover recovery, exhaustion, terminal phases, replacement identity, authentication/transport failures, timeout, and zero detaches during fresh onboarding. I found no important missing regression case requiring a merge hold.

• Skipped/canceled checks: Later CI runs intentionally skip tests for metadata-only edits. The canceled maintainer-edits job actually logged that its requirement was satisfied, and subsequent runs passed. I read both selected diagnostic logs completely. No current failed or pending check requires author repair, workflow approval, or rerunning. CodeRabbit’s docstring warning is advisory.