fix(onboard): attach selected inference provider on sandbox create by laitingsheng · Pull Request #7186 · NVIDIA/NemoClaw

laitingsheng · 2026-07-19T06:50:20Z

Summary

Onboarding registered the selected inference provider and configured its gateway route but never attached it during sandbox creation, so a Ready sandbox could have provider_count=0 and return HTTP 503 on inference.local until a manual sandbox provider attach, with rebuild recreating the broken state. The selected provider now flows through the serializable create intent and is emitted as a create-time provider attachment, so inference works without manual repair and reproduces deterministically across resume, retry, and rebuild.

Related Issue

Fixes #7171

Changes

Add a secret-free inferenceProvider field to the create-intent contract (SandboxCreateIntent, ResolveSandboxCreateIntentInput, CompleteSandboxCreateIntentInput) and carry it through intent resolution; the value normalizes to a trimmed provider name or null and is never a credential.
Pass the selected provider from the sandbox handler (this.options.provider) and the legacy/fallback create path (provider) into intent resolution.
Materialize the provider through one deterministic insertion-ordered set across the selected inference provider, messaging providers, the Hermes tool gateway, and extra providers, so --provider <selected> is emitted exactly once even when the name is duplicated elsewhere.
Keep the selected provider inside the durable create-intent fingerprint so resume, retry, and rebuild reproduce the attachment.

Type of Change

Code change (feature, bug fix, or refactor)
Code change with doc updates
Doc only (prose changes, no code sample modifications)
Doc only (includes code sample changes)

Quality Gates

Tests added or updated for changed behavior
Existing tests cover changed behavior — justification:
Tests not applicable — justification:
Docs updated for user-facing behavior changes
Docs not applicable — justification: internal create-intent plumbing repairing a defect; no user-facing command, flag, or documented behavior changes.
Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project cli src/lib/onboard/sandbox-create-plan.test.ts → 19 passed; npx vitest run --project cli src/lib/onboard/ → 3179 passed, 1 skipped, 1 pre-existing unrelated failure (docker-driver-gateway-jwt-bundle concurrency test, fails on a Node DEP0205 deprecation warning on subprocess stderr, untouched by this change).
Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
Quality Gates section completed with required justifications or waivers
No secrets, API keys, or credentials committed
npm run docs builds without warnings (doc changes only)
Doc pages follow the style guide (doc changes only)
New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

New Features
- Sandbox creation now supports selecting an inference provider.
- The selected provider is included in the sandbox configuration and reflected in provider arguments.
- Provider names are normalized (whitespace trimmed); blank selections are treated as unconfigured (null).
- Provider arguments are deduplicated so the selected provider isn’t repeated when already present via other providers.
- Provider credentials are excluded from serialized sandbox creation details.
Tests
- Added coverage for provider selection, normalization, deduplication, and unconfigured states.

Carry the selected inference provider through the serializable create intent so sandbox creation emits it as a create-time provider attachment. This keeps the gateway route backed by an attached provider on fresh, resume, retry, and rebuild, instead of leaving a Ready sandbox whose inference endpoint returns 503 until a manual attach. Fixes #7171 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

coderabbitai · 2026-07-19T06:50:47Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 93bca578-ef6d-464e-a2b0-a41f60f5ce3b

📥 Commits

Reviewing files that changed from the base of the PR and between 822310e and d23b750.

📒 Files selected for processing (3)

src/lib/onboard.ts
src/lib/onboard/machine/core-flow-phases.test.ts
src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts

🚧 Files skipped from review as they are similar to previous changes (3)

src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts
src/lib/onboard/machine/core-flow-phases.test.ts
src/lib/onboard.ts

📝 Walkthrough

Walkthrough

The selected inference provider now flows through sandbox create intent resolution, is normalized into durable intent state, and is emitted once in sandbox creation arguments alongside other providers. Tests cover trimming, omission, ordering, and deduplication.

Changes

Inference provider attachment

Layer / File(s)	Summary
Intent contract and normalization `src/lib/onboard/sandbox-create-intent-types.ts`, `src/lib/onboard/sandbox-create-intent-resolution.ts`, `src/lib/onboard/sandbox-create-intent.ts`	Intent inputs and outputs now carry `inferenceProvider`, which is trimmed and normalized to `null` when blank.
Create intent provider wiring `src/lib/onboard/machine/handlers/sandbox.ts`, `src/lib/onboard/sandbox-create-plan.ts`, `src/lib/onboard.ts`, `src/lib/onboard/machine/.../test`	Sandbox, plan, and legacy onboarding paths pass the selected provider into intent resolution, with updated mocks.
Provider argument materialization and validation `src/lib/onboard/sandbox-create-plan-materialization.ts`, `src/lib/onboard/sandbox-create-plan.test.ts`	Provider arguments are built from an insertion-ordered deduplicated set, with tests for serialization, ordering, omission, and duplicates.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

NVIDIA/NemoClaw#6218: Both changes modify the sandbox create intent and provider argument materialization flow.
NVIDIA/NemoClaw#6587: Both changes affect provider inputs used during sandbox create-plan generation.

Suggested labels: area: inference, area: providers, area: sandbox

Suggested reviewers: cv, prekshivyas, apurvvkumaria

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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 summarizes the main change: attaching the selected inference provider during sandbox creation.
Linked Issues check	✅ Passed	The changes thread inferenceProvider through intent flow, attach it at create time, deduplicate providers, and add focused coverage.
Out of Scope Changes check	✅ Passed	The diff stays focused on onboarding sandbox provider attachment and related tests, with no clear unrelated changes.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/attach-selected-inference-provider

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

github-code-quality · 2026-07-19T06:51:07Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/attach-selected-... branch remains at 80%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.

File	`main` 78e0e4e	`fix/attach-selected-...` d23b750	+/-
`src/lib/domain/.../connect-env.ts`	97%	89%	-8%
`src/lib/onboard.ts`	31%	30%	-1%
`src/lib/onboard...erialization.ts`	91%	91%	0%
`src/lib/onboard...reate-intent.ts`	97%	97%	0%
`src/lib/sandbox...rce-identity.ts`	91%	91%	0%
`src/lib/state/m...-acquisition.ts`	84%	84%	0%
`src/lib/state/m...ock-identity.ts`	95%	95%	0%
`src/lib/state/m...lock-storage.ts`	97%	97%	0%
`src/lib/inferen...er-lifecycle.ts`	65%	71%	+6%
`src/lib/inferen...lama/process.ts`	50%	100%	+50%

_{Updated July 19, 2026 08:00 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts (1)
126-133: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve inferenceProvider in the test fixture.

This mock always returns null, so handler tests using createDeps cannot prove that the selected provider reaches sandbox creation; a regression in provider wiring could still pass. Add inferenceProvider?: string | null to the mocked input and return input.inferenceProvider ?? null.
Proposed fix
       async (input: {
         sandboxName: string;
+        inferenceProvider?: string | null;
         extraProviders: readonly string[];
         staleExtraProviders: readonly string[];
       }) => ({
         sandboxName: input.sandboxName,
-        inferenceProvider: null,
+        inferenceProvider: input.inferenceProvider ?? null,
As per path instructions, tests must prove public entrypoints reach the new path and must not use broad mocks that bypass the behavior under test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts` around lines 126 -
133, Update the resolveCreateIntent mock input type in createDeps to include
optional inferenceProvider, and return input.inferenceProvider when provided,
falling back to null. Preserve the existing sandboxName and provider fields so
handler tests can verify the selected provider reaches sandbox creation.
Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/onboard/machine/core-flow-phases.test.ts`:
- Around line 217-220: Update the resolveSandboxCreateIntent mock to destructure
and return the received inferenceProvider instead of always setting it to null.
Extend the relevant core-flow test assertions to verify that createSandbox
observes the selected provider in the resolved intent.

---

Outside diff comments:
In `@src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts`:
- Around line 126-133: Update the resolveCreateIntent mock input type in
createDeps to include optional inferenceProvider, and return
input.inferenceProvider when provided, falling back to null. Preserve the
existing sandboxName and provider fields so handler tests can verify the
selected provider reaches sandbox creation.

🪄 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: Enterprise

Run ID: 895b83d7-24c2-43d9-ad13-19fb70ce1820

📥 Commits

Reviewing files that changed from the base of the PR and between 4deebcf and 822310e.

📒 Files selected for processing (10)

src/lib/onboard.ts
src/lib/onboard/machine/core-flow-phases.test.ts
src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts
src/lib/onboard/machine/handlers/sandbox.ts
src/lib/onboard/sandbox-create-intent-resolution.ts
src/lib/onboard/sandbox-create-intent-types.ts
src/lib/onboard/sandbox-create-intent.ts
src/lib/onboard/sandbox-create-plan-materialization.ts
src/lib/onboard/sandbox-create-plan.test.ts
src/lib/onboard/sandbox-create-plan.ts

github-actions · 2026-07-19T06:56:11Z

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions
Status: Canonical ledger: 0 blocker(s), 1 warning(s), 0 suggestion(s).

Model lanes

GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, onboard-repair, onboard-resume

2 optional E2E recommendations

sandbox-rebuild
inference-routing

1 warning · 0 suggestions

Warnings

Warnings do not block.

`PRA-1` Warning — Cover selected provider across resumed intent recomputation

Location: src/lib/onboard/machine/handlers/sandbox.ts:1119
Category: tests
Problem: The changed machine-flow caller now supplies `this.options.provider` when resolving the create intent, but the existing interruption/resume regression only compares successive resolver inputs wholesale and never exercises a non-default provider or asserts that the final `createSandbox` intent retains it. A future change can therefore drop the new field on the resume/retry path while the new plan-only tests continue to pass.
Impact: A resumed onboarding run after an interrupted intent resolution could recreate a sandbox without its selected inference provider, leaving inference unattached despite fresh-path coverage.
Recommendation: Extend the existing interrupted intent-resolution resume test with a non-default provider and assert that each resolver input has that provider and that the final create intent's resolved provider matches it.
Verification: Read `src/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.ts` around the interrupted-resolution test and confirm it currently asserts only equality of resolver inputs and resource profile, not `inferenceProvider` or the final resolved intent.
Test coverage: A machine-flow resume/retry test that interrupts `resolveCreateIntent` once, resumes with a non-default provider, and asserts both resolution calls plus `createSandbox`'s resolved intent contain that provider exactly once.
Evidence: `src/lib/onboard/machine/handlers/sandbox.ts:1119` adds `inferenceProvider: this.options.provider` to the create-intent call. `sandbox-create-intent-boundary.test.ts:475-502` retries after an interrupted resolver but does not assert the new inference-provider field or final resolved intent. The new `sandbox-create-plan.test.ts` cases exercise intent/materialization but not machine-flow resume/retry propagation.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

…nference-provider

…ocks Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

apurvvkumaria

Reviewed exact head d23b750.

The selected inference provider is carried as a normalized, secret-free name through both machine and fallback intent resolution, included in the durable fingerprint, and materialized via tokenized argv with deterministic deduplication across inference, messaging, Hermes-tool, and extra providers. I found no credential exposure, command-injection path, authorization regression, or sandbox/policy weakening.

Independent validation: 68 focused tests passed across create-plan, core-flow, interrupted-intent resume, recreate, checkpoint crash recovery, and prepared build-context coverage; npm run build:cli and npm run typecheck:cli passed. All commits are DCO-signed and GitHub-verified; CodeRabbit has no unresolved thread; exact-head CI, CodeQL, security scans, self-hosted checks, macOS E2E, and all three protected E2E selections passed.

I reviewed the advisor warning about making the interrupted-resume provider assertion more explicit. The current implementation recomputes the same provider on both attempts, the focused resume coverage passes, and the protected gate is green, so I do not consider that non-blocking test-hardening suggestion a merge blocker.

## Summary Add the canonical dated changelog entry for NemoClaw `v0.0.89` before the release plan captures the tag commit. The entry summarizes the user-visible Station preparation, inference, recovery, policy-disclosure, and CLI-containment changes merged since `v0.0.88`. ## Changes - Add `docs/changelog/2026-07-20.mdx` with the exact `## v0.0.89` release heading, parser-safe SPDX comment, summary, and detailed bullets. - Link each shipped theme to the most specific published OpenClaw documentation routes. - Keep internal E2E, advisory-registry, and review-workflow refactors out of the user-facing release summary. Source summary: - #7214, #7241, #7237, #7223, #7204, #7202, #7183, and #7090 -> `docs/changelog/2026-07-20.mdx`: Summarize qualified DGX Station identity, package-state, PackageKit, DKMS, and reboot-handoff fixes. - #7242, #7221, #7186, #7164, and #6874 -> `docs/changelog/2026-07-20.mdx`: Summarize inference endpoint provenance, provider attachment, managed vLLM cleanup and progress, and Ollama selection guidance. - #7225, #7216, #7192, #7136, #7096, and #6910 -> `docs/changelog/2026-07-20.mdx`: Summarize sandbox readiness, recovery guidance, rebuilt model routing, durable Hermes state, and gateway restart behavior. - #7187 -> `docs/changelog/2026-07-20.mdx`: Summarize complete effective messaging-preset egress disclosure before policy mutation. - #7218, #7165, and #7184 -> `docs/changelog/2026-07-20.mdx`: Summarize structured output containment, terminal-safe route display, and Hermes forward cleanup. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This PR adds release-history prose only; the focused changelog contract test validates its required structure and routes. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `mise exec node@22.23.1 -- npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only release entry. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — `mise exec node@22.23.1 -- npm run docs` completed with 0 errors and 2 existing site-wide warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and does not use frontmatter. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>  ## Summary by CodeRabbit * **New Features** * Expanded DGX Station installation support for qualified GB300 and OTA-upgraded environments. * Preserved selected inference providers, endpoints, model pins, and tuning settings during sandbox creation and rebuilds. * Improved sandbox recovery by validating availability and stability before restarting services. * Added clearer policy output showing the complete effective messaging egress configuration. * **Bug Fixes** * Hardened status and inference route displays by sanitizing terminal control characters. * Improved Hermes uninstall behavior by stopping detached dashboard forwarding.  Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

coderabbitai Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread src/lib/onboard/machine/core-flow-phases.test.ts Outdated

laitingsheng added bug-fix PR fixes a bug or regression area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow labels Jul 19, 2026

laitingsheng added 2 commits July 19, 2026 07:44

Merge remote-tracking branch 'origin/main' into fix/attach-selected-i…

f526f2e

…nference-provider

fix(onboard): satisfy growth guardrail and forward provider in test m…

d23b750

…ocks Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

laitingsheng added the v0.0.87 label Jul 19, 2026

apurvvkumaria self-assigned this Jul 19, 2026

apurvvkumaria approved these changes Jul 19, 2026

View reviewed changes

coderabbitai Bot mentioned this pull request Jul 19, 2026

feat(policy): support persistent baseline network policy exclusions #7194

Merged

23 tasks

laitingsheng added v0.0.89 and removed v0.0.87 labels Jul 19, 2026

cv merged commit c912f40 into main Jul 19, 2026
76 checks passed

cv deleted the fix/attach-selected-inference-provider branch July 19, 2026 13:52

ericksoa mentioned this pull request Jul 20, 2026

docs: add v0.0.89 release changelog #7255

Merged

21 tasks

jyaunches mentioned this pull request Jul 25, 2026

[Brev][Policy&Network] Brev launchable sandbox container cannot resolve inference.local after compatible-endpoint onboard #7433

Closed

Navigation Menu

Uh oh!

fix(onboard): attach selected inference provider on sandbox create - #7186

fix(onboard): attach selected inference provider on sandbox create#7186
cv merged 3 commits into
mainfrom
fix/attach-selected-inference-provider

laitingsheng commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-code-quality Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 19, 2026 •

edited

Loading

Warnings

`PRA-1` Warning — Cover selected provider across resumed intent recomputation

Uh oh!

apurvvkumaria left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

laitingsheng commented Jul 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Quality Gates

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-code-quality Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor — Informational

Model lanes

E2E guidance

Warnings

PRA-1 Warning — Cover selected provider across resumed intent recomputation

Uh oh!

apurvvkumaria left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

laitingsheng commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

github-code-quality Bot commented Jul 19, 2026 •

edited

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading

`PRA-1` Warning — Cover selected provider across resumed intent recomputation