Skip to content

refactor(onboard): separate sandbox intent from effects - #6218

Merged
cv merged 3 commits into
mainfrom
refactor/onboard-plan-effects
Jul 3, 2026
Merged

refactor(onboard): separate sandbox intent from effects#6218
cv merged 3 commits into
mainfrom
refactor/onboard-plan-effects

Conversation

@cv

@cv cv commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Separates create-time onboarding intent resolution from effectful materialization while preserving the existing prepareSandboxCreatePlan entry point and runtime behavior. This establishes a typed internal seam for later FSM work without persisting temporary paths, cleanup callbacks, or messaging credential values.

Changes

  • Add a deterministic SandboxCreateIntent resolver with explicit credential metadata, policy inputs, GPU arguments, and provider contributions.
  • Materialize temporary policy files, resource flags, provider cleanup/upserts, and concrete create arguments in a separate effectful phase.
  • Keep the serializable intent contract in a dedicated type-only module so the execution path remains focused.
  • Reject changed credential availability or provider type before any materialization effects run.
  • Preserve provider ordering, channel filtering, policy-tier behavior, and the existing compatibility wrapper.
  • Add characterization coverage for serialization, credential-value exclusion, effect ordering, stale bindings, disabled channels, GPU behavior, and provider deduplication.
  • Keep FSM/session capture out of scope: recreate still reaches this seam after the existing destructive boundary, which requires a separate migration.
  • Local verification used Node.js 22: 39 focused tests pass, CLI typecheck/build and repository checks pass, and GitHub verifies all three signed commits. The repository-wide test-cli hook was attempted, then skipped for the commits because macOS lacks GNU script -qec and Docker; a separate CLI run reached 5,335 passing tests with 18 unrelated timeout failures in untouched files.

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 behavior-preserving refactor with no CLI, prompt, output, persistence, event, or public API change.
  • 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: reviewed credential-value exclusion, binding validation, and side-effect ordering; focused tests assert no effects run for stale bindings.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened validation during sandbox creation so mismatched or missing messaging credential bindings are detected early, preventing partial side effects.
    • Improved determinism and correctness of messaging provider request generation and the resulting provider wiring order.
  • Refactor

    • Reworked sandbox creation into a two-stage flow: deriving a serializable “intent” and then materializing it into final creation arguments and providers.
    • Updated messaging provider and active channel resolution to be request-driven, with consistent ordering and Hermes gateway integration.
  • Tests

    • Expanded and added coverage for intent resolution, credential-binding failure behavior, deterministic output, and effect ordering.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Refactors sandbox creation into a two-stage flow that resolves a serializable create intent and then materializes it into create arguments, provider wiring, and credential validation.

Changes

Sandbox create-plan two-stage refactor

Layer / File(s) Summary
Intent and materialization types
src/lib/onboard/sandbox-create-intent-types.ts
Adds exported intent, request, policy, and materialization input types.
Provider-request and intent resolution
src/lib/onboard/sandbox-create-plan.ts
Reworks provider-channel mapping and active channel selection, then builds the serializable sandbox create intent from messaging provider requests and policy options.
Materialization and plan wiring
src/lib/onboard/sandbox-create-plan.ts
Adds credential binding checks, materializes create arguments and provider wiring, and rewires prepareSandboxCreatePlan to use the new two-stage flow.
Test coverage updates
src/lib/onboard/sandbox-create-plan.test.ts
Updates imports, adds intent and materialization tests, and expands existing fixtures to the full messaging token definition shape.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant prepareSandboxCreatePlan
  participant resolveSandboxCreateIntent
  participant materializeSandboxCreatePlan
  participant upsertMessagingProviders

  prepareSandboxCreatePlan->>resolveSandboxCreateIntent: build intent inputs
  resolveSandboxCreateIntent-->>prepareSandboxCreatePlan: SandboxCreateIntent
  prepareSandboxCreatePlan->>materializeSandboxCreatePlan: materialize intent with token defs
  materializeSandboxCreatePlan->>upsertMessagingProviders: create provider records
  upsertMessagingProviders-->>materializeSandboxCreatePlan: provider names
  materializeSandboxCreatePlan-->>prepareSandboxCreatePlan: SandboxCreatePlan
Loading

Suggested labels: chore

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactor: separating sandbox create intent from effectful materialization.
✨ 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 refactor/onboard-plan-effects

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

@github-code-quality

github-code-quality Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the refactor/onboard-pla... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/onboard-pla... 1329986 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/blueprint/ssrf.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the refactor/onboard-pla... branch is 69%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/onboard-pla... 1329986 +/-
src/lib/actions...dbox/rebuild.ts 82%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/shields/index.ts 75%
src/lib/state/sandbox.ts 73%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/policy/index.ts 56%
src/lib/onboard.ts 20%

Updated July 03, 2026 00:01 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 4 suggestions · 6 test follow-ups
Since last review: 3 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Add test for providerType undefined vs missing equivalence in intent resolution
  • PRA-T5 Add or justify test follow-up: Add test for extraProviders empty-string filtering in intent
  • PRA-T6 Add or justify test follow-up: Add test for policyTier validation against KNOWN_POLICY_TIER_NAMES in intent
  • PRA-1 In-scope improvement: Add test for providerType undefined vs missing equivalence in intent resolution in src/lib/onboard/sandbox-create-plan.test.ts:1
  • PRA-2 In-scope improvement: Add test for extraProviders empty-string filtering in intent in src/lib/onboard/sandbox-create-plan.test.ts:1
  • PRA-3 In-scope improvement: Add test for policyTier validation against KNOWN_POLICY_TIER_NAMES in intent in src/lib/onboard/sandbox-create-plan.test.ts:1
  • PRA-4 In-scope improvement: Monolith growth justified by structural decomposition — types extracted, functions decomposed in src/lib/onboard/sandbox-create-plan.ts:1

Findings index

ID Severity Category Location Required action
PRA-1 Improvement tests src/lib/onboard/sandbox-create-plan.test.ts:1 Add a focused test in the `resolveSandboxCreateIntent` describe block asserting that a request with `providerType: undefined` and one with omitted `providerType` produce identical intent output.
PRA-2 Improvement tests src/lib/onboard/sandbox-create-plan.test.ts:1 Add a test case in `resolves deterministic serializable intent` or new test passing `extraProviders: ['a', '', 'b']` and asserting `intent.extraProviders` equals `['a', 'b']`.
PRA-3 Improvement tests src/lib/onboard/sandbox-create-plan.test.ts:1 Add a test asserting that `resolveSandboxCreateIntent` preserves the passed `policyTier` value (including invalid ones) since validation is caller responsibility — or add validation if intent should be self-protecting.
PRA-4 Improvement architecture src/lib/onboard/sandbox-create-plan.ts:1 No action needed. Growth is justified architectural improvement. Monitor if file exceeds ~600 lines in future PRs.
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 4 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

  • None.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Add test for providerType undefined vs missing equivalence in intent resolution

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: The `bindMessagingTokenDefs` normalization (`tokenDef.providerType || undefined`) is tested in materialization, but intent resolution (`resolveSandboxCreateIntent`) does not have a test verifying that missing and undefined providerType are treated equivalently when building the intent.
  • Impact: Low — logic is correct and tested downstream, but gap in intent-level characterization could allow future regression if intent construction changes.
  • Suggested action: Add a focused test in the `resolveSandboxCreateIntent` describe block asserting that a request with `providerType: undefined` and one with omitted `providerType` produce identical intent output.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -A 20 'resolves deterministic serializable intent' src/lib/onboard/sandbox-create-plan.test.ts — look for providerType equivalence assertion.
  • Missing regression test: Test: 'treats missing and undefined providerType as equivalent in resolved intent' — construct two inputs differing only in providerType presence, assert resolved intents equal.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: bindMessagingTokenDefs normalizes with `const boundProviderType = tokenDef.providerType || undefined;` (line 215) but no intent-level test covers this equivalence.

PRA-2 Improvement — Add test for extraProviders empty-string filtering in intent

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: Intent construction filters `extraProviders` with `.filter(Boolean)` (line 174), but no test verifies empty strings are removed.
  • Impact: Low — defensive filtering, unlikely to regress, but untested.
  • Suggested action: Add a test case in `resolves deterministic serializable intent` or new test passing `extraProviders: ['a', '', 'b']` and asserting `intent.extraProviders` equals `['a', 'b']`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -n 'extraProviders' src/lib/onboard/sandbox-create-plan.test.ts — confirm empty-string filtering is asserted.
  • Missing regression test: Test: 'filters empty strings from extraProviders in resolved intent' — pass extraProviders with empty strings, assert they are absent from intent.extraProviders.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 174: `extraProviders: [...new Set(extraProviders ?? [])].filter(Boolean)` — no test assertion for this behavior.

PRA-3 Improvement — Add test for policyTier validation against KNOWN_POLICY_TIER_NAMES in intent

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: The `readPolicyTierEnv` validates tier against known set, but `resolveSandboxCreateIntent` accepts any string for `policyTier` without validation. If an invalid tier reaches intent, it will propagate to policy.
  • Impact: Low — `prepareSandboxCreatePlan` calls `readPolicyTierEnv` which validates, but intent function could be called directly by future FSM code.
  • Suggested action: Add a test asserting that `resolveSandboxCreateIntent` preserves the passed `policyTier` value (including invalid ones) since validation is caller responsibility — or add validation if intent should be self-protecting.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -n 'policyTier' src/lib/onboard/sandbox-create-plan.test.ts — check if invalid tier values are tested.
  • Missing regression test: Test: 'preserves arbitrary policyTier value in intent for caller validation' — pass invalid tier, assert it appears in intent.policy.options.policyTier.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 178: `policyTier` passed through to intent.policy.options without validation; readPolicyTierEnv validates at caller boundary only.

PRA-4 Improvement — Monolith growth justified by structural decomposition — types extracted, functions decomposed

  • Location: src/lib/onboard/sandbox-create-plan.ts:1
  • Category: architecture
  • Problem: File grew from 311 to 467 lines (+156) but 80 lines of types extracted to sandbox-create-intent-types.ts. Growth is from 3 new exported functions (resolveSandboxCreateMessagingProviderRequests, resolveSandboxCreateIntent, materializeSandboxCreatePlan) with explicit logic, not feature bloat. Enables independent testing, TOCTOU guards, and future FSM work.
  • Impact: Positive — larger file but better architecture. Types reusable, intent testable without effects, credential-binding validation explicit.
  • Suggested action: No action needed. Growth is justified architectural improvement. Monitor if file exceeds ~600 lines in future PRs.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: wc -l src/lib/onboard/sandbox-create-plan.ts src/lib/onboard/sandbox-create-intent-types.ts — confirm combined lines ~554 vs original 311.
  • Missing regression test: N/A — architecture concern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Types extracted (87 lines), 3 new pure functions with comprehensive tests, TOCTOU protection added, prepareSandboxCreatePlan preserved as compatibility wrapper.
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-4 shrink (src/lib/onboard/sandbox-create-plan.ts:1): Monolith threshold exceeded (+156 lines) but offset by type extraction (-80 lines) and justified by decomposition.
    • Replacement: Keep as-is; architectural improvement enables testability and security guards.
    • Net: 76 lines
    • Safety boundary: All security validation, credential handling, and effect ordering preserved; types pure TS interfaces.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add test for providerType undefined vs missing equivalence in intent resolution. Internal refactor with behavior preservation; unit tests comprehensively cover new functions (13 test blocks) including negative TOCTOU cases. Runtime/integration validation recommended for full onboard flow but not required for this PR since prepareSandboxCreatePlan wrapper is unchanged and existing CLI tests cover runtime.
  • PRA-T2 Runtime validation — Add test for extraProviders empty-string filtering in intent. Internal refactor with behavior preservation; unit tests comprehensively cover new functions (13 test blocks) including negative TOCTOU cases. Runtime/integration validation recommended for full onboard flow but not required for this PR since prepareSandboxCreatePlan wrapper is unchanged and existing CLI tests cover runtime.
  • PRA-T3 Runtime validation — Add test for policyTier passthrough validation boundary in intent. Internal refactor with behavior preservation; unit tests comprehensively cover new functions (13 test blocks) including negative TOCTOU cases. Runtime/integration validation recommended for full onboard flow but not required for this PR since prepareSandboxCreatePlan wrapper is unchanged and existing CLI tests cover runtime.
  • PRA-T4 Add test for providerType undefined vs missing equivalence in intent resolution — Add a focused test in the `resolveSandboxCreateIntent` describe block asserting that a request with `providerType: undefined` and one with omitted `providerType` produce identical intent output.
  • PRA-T5 Add test for extraProviders empty-string filtering in intent — Add a test case in `resolves deterministic serializable intent` or new test passing `extraProviders: ['a', '', 'b']` and asserting `intent.extraProviders` equals `['a', 'b']`.
  • PRA-T6 Add test for policyTier validation against KNOWN_POLICY_TIER_NAMES in intent — Add a test asserting that `resolveSandboxCreateIntent` preserves the passed `policyTier` value (including invalid ones) since validation is caller responsibility — or add validation if intent should be self-protecting.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Add test for providerType undefined vs missing equivalence in intent resolution

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: The `bindMessagingTokenDefs` normalization (`tokenDef.providerType || undefined`) is tested in materialization, but intent resolution (`resolveSandboxCreateIntent`) does not have a test verifying that missing and undefined providerType are treated equivalently when building the intent.
  • Impact: Low — logic is correct and tested downstream, but gap in intent-level characterization could allow future regression if intent construction changes.
  • Suggested action: Add a focused test in the `resolveSandboxCreateIntent` describe block asserting that a request with `providerType: undefined` and one with omitted `providerType` produce identical intent output.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -A 20 'resolves deterministic serializable intent' src/lib/onboard/sandbox-create-plan.test.ts — look for providerType equivalence assertion.
  • Missing regression test: Test: 'treats missing and undefined providerType as equivalent in resolved intent' — construct two inputs differing only in providerType presence, assert resolved intents equal.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: bindMessagingTokenDefs normalizes with `const boundProviderType = tokenDef.providerType || undefined;` (line 215) but no intent-level test covers this equivalence.

PRA-2 Improvement — Add test for extraProviders empty-string filtering in intent

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: Intent construction filters `extraProviders` with `.filter(Boolean)` (line 174), but no test verifies empty strings are removed.
  • Impact: Low — defensive filtering, unlikely to regress, but untested.
  • Suggested action: Add a test case in `resolves deterministic serializable intent` or new test passing `extraProviders: ['a', '', 'b']` and asserting `intent.extraProviders` equals `['a', 'b']`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -n 'extraProviders' src/lib/onboard/sandbox-create-plan.test.ts — confirm empty-string filtering is asserted.
  • Missing regression test: Test: 'filters empty strings from extraProviders in resolved intent' — pass extraProviders with empty strings, assert they are absent from intent.extraProviders.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 174: `extraProviders: [...new Set(extraProviders ?? [])].filter(Boolean)` — no test assertion for this behavior.

PRA-3 Improvement — Add test for policyTier validation against KNOWN_POLICY_TIER_NAMES in intent

  • Location: src/lib/onboard/sandbox-create-plan.test.ts:1
  • Category: tests
  • Problem: The `readPolicyTierEnv` validates tier against known set, but `resolveSandboxCreateIntent` accepts any string for `policyTier` without validation. If an invalid tier reaches intent, it will propagate to policy.
  • Impact: Low — `prepareSandboxCreatePlan` calls `readPolicyTierEnv` which validates, but intent function could be called directly by future FSM code.
  • Suggested action: Add a test asserting that `resolveSandboxCreateIntent` preserves the passed `policyTier` value (including invalid ones) since validation is caller responsibility — or add validation if intent should be self-protecting.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: grep -n 'policyTier' src/lib/onboard/sandbox-create-plan.test.ts — check if invalid tier values are tested.
  • Missing regression test: Test: 'preserves arbitrary policyTier value in intent for caller validation' — pass invalid tier, assert it appears in intent.policy.options.policyTier.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 178: `policyTier` passed through to intent.policy.options without validation; readPolicyTierEnv validates at caller boundary only.

PRA-4 Improvement — Monolith growth justified by structural decomposition — types extracted, functions decomposed

  • Location: src/lib/onboard/sandbox-create-plan.ts:1
  • Category: architecture
  • Problem: File grew from 311 to 467 lines (+156) but 80 lines of types extracted to sandbox-create-intent-types.ts. Growth is from 3 new exported functions (resolveSandboxCreateMessagingProviderRequests, resolveSandboxCreateIntent, materializeSandboxCreatePlan) with explicit logic, not feature bloat. Enables independent testing, TOCTOU guards, and future FSM work.
  • Impact: Positive — larger file but better architecture. Types reusable, intent testable without effects, credential-binding validation explicit.
  • Suggested action: No action needed. Growth is justified architectural improvement. Monitor if file exceeds ~600 lines in future PRs.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run: wc -l src/lib/onboard/sandbox-create-plan.ts src/lib/onboard/sandbox-create-intent-types.ts — confirm combined lines ~554 vs original 311.
  • Missing regression test: N/A — architecture concern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Types extracted (87 lines), 3 new pure functions with comprehensive tests, TOCTOU protection added, prepareSandboxCreatePlan preserved as compatibility wrapper.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard, messaging-providers
Optional E2E: full-e2e, onboard-resume, onboard-repair

Dispatch hint: cloud-onboard,messaging-providers

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard (high; live Docker/OpenShell sandbox with hosted inference, 70 minute timeout): This is a runtime onboarding/sandbox-create path change. cloud-onboard validates a full hosted non-interactive onboard against a live sandbox, including initial policy generation, create args, OpenShell sandbox creation, and hosted inference readiness.
  • messaging-providers (high; live Docker/OpenShell sandbox with messaging provider and hosted inference checks, 90 minute timeout): The changed code directly affects messaging provider requests, credential binding/materialization, disabled-channel filtering, provider upsert inputs, and policy channel activation. messaging-providers is the live E2E that exercises provider creation, credential isolation, OpenClaw config patching, network reachability, and token/proxy boundaries for messaging channels.

Optional E2E

  • full-e2e (high; live Docker/OpenShell sandbox with hosted inference, 75 minute timeout): Useful broader confidence for the complete OpenClaw user journey after sandbox-create plan refactoring, including live inference and CLI operations. Not strictly required because cloud-onboard covers the hosted onboarding path and messaging-providers covers the credential/provider-specific risk.
  • onboard-resume (medium-high; live Docker/OpenShell resume scenario, 45 minute timeout): Adjacent confidence if reviewers believe the new serializable sandbox-create intent will be used across interrupted onboarding boundaries. The touched files are not the machine live-slice/resume state code, so the onboarding resume rule does not make this merge-blocking.
  • onboard-repair (high; live Docker/OpenShell repair scenario, 75 minute timeout): Adjacent confidence for repair/recreate behavior if the sandbox-create materialization changes interact with repairing an existing sandbox. Not required because this PR does not touch onboard machine repair policy or resume state transitions.

New E2E recommendations

  • sandbox-create-intent-secret-boundary (medium): Existing live E2E exercises the resulting sandbox and messaging providers, but does not appear to directly assert that a serialized sandbox-create intent crossing a process or resume boundary is secretless and fails closed on changed credential bindings. If this intent becomes persisted or emitted outside the current process, add a targeted E2E for that boundary.
    • Suggested test: Add a live or workflow-dispatched sandbox-create intent boundary test that interrupts after intent resolution, inspects persisted/emitted state for absence of token values, then verifies materialization rejects missing or changed credential bindings before side effects.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: cloud-onboard,messaging-providers

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw, ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E targets: ubuntu-repo-docker-post-reboot-recovery

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: The PR refactors create-time sandbox planning, including provider bindings, active messaging channel resolution, policy options, GPU create args, and materialization side effects. The baseline Ubuntu cloud OpenClaw target is live-supported and exercises the normal sandbox creation path affected by these changes.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw
  • ubuntu-repo-cloud-langchain-deepagents-code: The create-plan changes carry agentName and policy-tier inputs into initial sandbox policy creation. The live-supported LangChain Deep Agents Code target exercises the agent-specific onboarding/policy path in addition to the shared sandbox creation machinery.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Optional E2E targets

  • ubuntu-repo-docker-post-reboot-recovery: Optional adjacent coverage: this target runs the same live-supported Ubuntu cloud OpenClaw onboarding path before its post-reboot recovery lifecycle, adding confidence that the refactored sandbox creation plan still leaves recoverable host/container state.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

  • src/lib/onboard/sandbox-create-intent-types.ts
  • src/lib/onboard/sandbox-create-plan.ts

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 3 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Onboard recreate with a messaging token preserves provider argv order across the `prepareSandboxCreatePlan` caller boundary: upserted messaging providers, reusable providers, Hermes tool provider, then extra providers.. Unit coverage is strong for the new pure resolver/materializer seam, but the changed source participates in sandbox onboarding, policy creation, provider registration, and concrete create argv construction, so targeted runtime/integration validation at the real caller boundary would add confidence.
  • PRA-T2 Runtime validation — Onboard stale credential availability aborts before temporary policy creation, resource flag mutation, provider pre-delete cleanup, or messaging provider upsert.. Unit coverage is strong for the new pure resolver/materializer seam, but the changed source participates in sandbox onboarding, policy creation, provider registration, and concrete create argv construction, so targeted runtime/integration validation at the real caller boundary would add confidence.
  • PRA-T3 Runtime validation — Onboard Brave providerType drift aborts before provider cleanup or upsert while the secretless intent remains free of `BRAVE_API_KEY` values.. Unit coverage is strong for the new pure resolver/materializer seam, but the changed source participates in sandbox onboarding, policy creation, provider registration, and concrete create argv construction, so targeted runtime/integration validation at the real caller boundary would add confidence.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/sandbox-create-plan.ts (1)

229-241: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Use the Set for primary credential lookup.

Line 241 calls .has() on primaryMessagingCredentialEnvKeys, which is typed as readonly string[]; this fails type-checking. Use the primaryCredentialEnvKeys set created on Line 229.

Proposed fix
-            return channel && primaryMessagingCredentialEnvKeys.has(envKey) ? [channel] : [];
+            return channel && primaryCredentialEnvKeys.has(envKey) ? [channel] : [];
🤖 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/sandbox-create-plan.ts` around lines 229 - 241, The primary
credential lookup in sandbox-create-plan is using the wrong type for membership
checks: `primaryMessagingCredentialEnvKeys` is a readonly array, so the `.has()`
call in the `messagingProviderRequests` filter will not type-check. Update the
`filterEnabledChannelNames` path to use the existing `primaryCredentialEnvKeys`
Set created in `sandbox-create-plan` for the `envKey` membership test, and keep
the `resolveQrSelectedChannels` flow unchanged.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@src/lib/onboard/sandbox-create-plan.ts`:
- Around line 229-241: The primary credential lookup in sandbox-create-plan is
using the wrong type for membership checks: `primaryMessagingCredentialEnvKeys`
is a readonly array, so the `.has()` call in the `messagingProviderRequests`
filter will not type-check. Update the `filterEnabledChannelNames` path to use
the existing `primaryCredentialEnvKeys` Set created in `sandbox-create-plan` for
the `envKey` membership test, and keep the `resolveQrSelectedChannels` flow
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 07d56891-0959-491f-bdcb-0e5e8424b731

📥 Commits

Reviewing files that changed from the base of the PR and between e33c093 and 06167cc.

📒 Files selected for processing (2)
  • src/lib/onboard/sandbox-create-plan.test.ts
  • src/lib/onboard/sandbox-create-plan.ts

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ⚠️ Run cancelled — no signal

Run: 28628873360
Workflow ref: refactor/onboard-plan-effects
Requested targets: ubuntu-repo-cloud-openclaw,ubuntu-repo-cloud-langchain-deepagents-code
Requested jobs: (default — all default-enabled free-standing jobs; explicit-only jobs hermes-gpu-startup, openshell-gateway-auth-contract, jetson-nvmap-gpu, and sandbox-rlimits-connect are skipped unless selected)
Summary: 0 passed, 0 failed, 1 cancelled, 0 skipped

Job Result
live ⚠️ cancelled

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ⚠️ Some jobs cancelled — partial pass

Run: 28628872231
Workflow ref: refactor/onboard-plan-effects
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,messaging-providers,full-e2e
Summary: 1 passed, 0 failed, 2 cancelled, 0 skipped

Job Result
cloud-onboard ⚠️ cancelled
full-e2e ✅ success
messaging-providers ⚠️ cancelled

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All selected jobs passed

Run: 28628983888
Workflow ref: refactor/onboard-plan-effects
Requested targets: ubuntu-repo-cloud-openclaw,ubuntu-repo-cloud-langchain-deepagents-code
Requested jobs: (default — all default-enabled free-standing jobs; explicit-only jobs hermes-gpu-startup, openshell-gateway-auth-contract, jetson-nvmap-gpu, and sandbox-rlimits-connect are skipped unless selected)
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
live ✅ success

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All requested jobs passed

Run: 28629117819
Workflow ref: refactor/onboard-plan-effects
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,messaging-providers
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
messaging-providers ✅ success

@cv

cv commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Automated-review follow-up for head 1329986c978b5f03111d5bf59dd6647b956bbb79:

  • The requested runtime coverage passed on the final head: cloud-onboard and messaging-providers, plus both required typed targets (OpenClaw and LangChain Deep Agents).
  • The credential-binding drift paths are deliberately covered at the unit seam: missing bindings, changed availability, and changed provider type all assert that policy/resource/cleanup/upsert effects have not run. A live flow cannot deterministically create that TOCTOU condition because the compatibility wrapper resolves and immediately materializes.
  • Nemotron's optional intent-characterization suggestions are left as-is: the success path already exercises an omitted providerType and the drift path covers a changed value; empty extraProviders filtering predates this refactor while the compatibility test covers ordering and deduplication; and policy-tier validation remains at the caller boundary (readPolicyTierEnv), so an invalid-tier passthrough test would lock in behavior this internal intent contract does not promise.

Both advisors now report no required findings or warnings.

@cv
cv merged commit 2a68fbc into main Jul 3, 2026
204 checks passed
@cv
cv deleted the refactor/onboard-plan-effects branch July 3, 2026 05:12
cv added a commit that referenced this pull request Jul 3, 2026
<!-- markdownlint-disable MD041 -->
## Summary
<!-- 1-3 sentences: what this PR does and why. -->

Prevents a Ready LangChain Deep Agents Code sandbox from being deleted
until its recorded OpenShell gateway, live inference route, managed base
image, and staged build context have been validated and sealed. A
DCode-only, process-local orchestration facade carries that verified
gateway/context pair through recreation, fixing the post-delete
port-8080 false conflict from #6195 without adding generalized persisted
replay or FSM state.

## Related Issue
<!-- Fixes #NNN or Closes #NNN. Remove this section if none. -->

Fixes #6195
Refs #6218
Refs #6224
Refs #6226
Refs #5801

## Changes
<!-- Bullet list of key changes. -->

- Bind DCode rebuilds to the sandbox's recorded gateway before
credential validation, probe the live `https://inference.local` route
without exposing credentials, and revalidate the gateway, registry
entry, route, schema, base image, and staged context immediately before
deletion.
- Build a disposable DCode image and fingerprint its managed build
context before mutation, then pass the retained context and recorded
gateway as a one-shot handoff through the create-intent seam landed in
#6218. Ordinary onboarding and non-DCode rebuild behavior remain
unchanged.
- Put the DCode lifecycle in `rebuild-dcode-orchestrator.ts`: it owns
scoped gateway cleanup, target preflight, replacement preparation,
mutation-edge revalidation, recovery behavior, and one-shot handoff
cleanup while depending on injected generic rebuild callbacks. This
reduced `rebuild.ts` from 1,599 to 1,460 lines and split its 1,477-line
flow spec into focused generic and DCode suites plus a shared harness.
- Extract the onboarding handoff into `prepared-dcode-rebuild.ts`, which
owns gateway validation, ordinary-versus-prepared staging/patching, and
consume-before-call semantics. The top-level `onboard.ts` entrypoint is
four lines smaller than `main`.
- Preserve the existing sandbox when initial preflight fails; if
mutation-edge validation fails after backup, keep the harmless backup
but do not delete or recreate the sandbox. Prepared-backup recovery
skips unavailable live-route probes and does not take a second backup.
Messaging-preflight aborts also restore the prior process gateway.
- Harden managed-context sealing against symlink substitution, pathname
replacement, and in-place mutation by reading regular files through
`O_NOFOLLOW` file descriptors and checking identity/metadata before and
after each read.
- Add a live DCode lifecycle fixture that rotates the credential stored
in the real gateway, waits for a 401, runs rebuild without a host
credential, and proves there was no backup/delete/create while container
identity, readiness, and a workspace marker remain intact.
- Document the DCode rebuild preflight and backup boundary in the
quickstart. A documentation review confirmed no further user-facing
changes are needed.
- Keep generalized FSM/session capture and cross-agent replay out of
scope for #6224/#6226; true build/health/swap replacement remains #5801.

## Type of Change

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent final
architecture and nine-category security reviews both passed the narrowed
DCode-only design with no blocker; the security review included the
final orchestration extraction and managed-context TOCTOU hardening.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [x] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

Commands and evidence:

- `make check` — passed on the final narrowed implementation under the
repository-expected `umask 022`.
- `npm test` — 998 files passed, 2 platform suites skipped; 11,277 tests
passed, 35 skipped.
- Post-merge focused/current-main set — 96/96 passed across generic
rebuild flow, DCode flow, managed-image preflight, prepared handoff,
inference-provider compatibility, and messaging setup; the final
descriptor-pinning, target-guard, and fail-closed review deltas passed
27/27 focused cases.
- Additional affected environment-failure audit — 121/121 passed; E2E
support — 26/26 passed.
- `npm run build:cli` and `npm run typecheck:cli` — passed.
- Current-tree Vitest project-membership check — 1,075 files across
eight disjoint projects; source-shape, test-file-size, and changed-test
conditional budgets passed.
- `npm run docs` — passed with 0 errors and 2 pre-existing warnings.
- Pre-push plugin and CLI TypeScript gates — passed.
- GitHub commit verification — 18/18 commits report `verified=true`,
reason `valid`.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added support for a new DCode rebuild flow during onboarding and
sandbox recreation.
* Introduced a preflight check for inference routing before rebuilds
proceed.

* **Bug Fixes**
* Rebuilds now fail safely when credentials or routing are invalid,
helping prevent unsafe sandbox changes.
* Improved rebuild handling so existing sandboxes and backups are
preserved if a later check fails.

* **Documentation**
* Updated the quickstart guide with clearer rebuild and backup behavior
for managed DCode sandboxes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv mentioned this pull request Jul 3, 2026
21 tasks
cv added a commit that referenced this pull request Jul 4, 2026
## Summary

Hardens the managed-MCP work merged in #5876 so DCode rebuilds validate
every reconstructable input before crossing the destructive delete
boundary, preserve exact policy intent, and migrate legacy managed MCP
state fail-closed. Prepared rebuild artifacts and the derived MCP
runtime snapshot remain ephemeral and process-local; neither is
persisted in FSM or checkpoint state, so this does not implement #6224.

## Related Issue

Refs #5876
Refs #6195
Refs #6218

## Changes

- Revalidate DCode route, image, Dockerfile, reasoning, web-search, and
MCP inputs after preparation and before NIM stop or sandbox deletion;
restore MCP state and relock shields on failure.
- Preserve exact custom network policy replay while keeping generated
MCP rules under the MCP adapter's exclusive ownership.
- Add protocol-specific policy schema validation for REST, WebSocket,
JSON-RPC, and MCP matchers, including cross-rule `tools/call` conflict
rejection.
- Pin Deep Agents Code 0.1.30 and load only a strict, canonicalized
managed MCP projection from a process-local integrity-bound snapshot.
Sealed memfd is preferred; when OpenShell seccomp blocks it, an
anonymous `O_TMPFILE` inode is reopened read-only and bound by
descriptor, device, inode, size, kind, and SHA-256, with ambient
discovery disabled.
- Bind the canonical TypeScript secret-pattern source and flags to one
shared behavior corpus executed through the Bash and Python DCode
enforcement boundaries, including the full ECMAScript whitespace set.
- Add capability-v2 gating and legacy-v1 teardown/rollback that
preserves unrelated user configuration and fails closed on malformed,
unsafe, or drifted state.
- Add rebuild, migration, runtime-patch, schema, snapshot, and lifecycle
coverage; update the MCP, policy, security, command, and DCode
documentation.

Verification notes:

- Final DCode-adjacent run: 9 files, 187 tests passed; the focused
descriptor/projection run passed 4 files and 138 tests.
- Final review-follow-up run: 82 focused Bash/Python/TypeScript parity
and descriptor-fallback tests passed, including all 25 ECMAScript
whitespace code points under both `C` and `C.UTF-8` Bash locales.
- Full pre-squash-equivalent run: 1,068 files passed, 2 skipped; 12,149
tests passed, 35 skipped.
- CLI coverage ratchet passed with the repository include/exclude set
expressed as one Vitest glob: lines 65.24%, statements 64.45%, functions
67.06%, branches 57.21%.
- Python compile, Biome, ShellCheck, shfmt, source-shape, test-size,
repository, secret-scan, and diff checks passed. The normal push hook
passed CLI typechecking.
- Main-sync validation after merging #6265 passed: 9 CLI files/82 tests,
6 integration files/174 tests, an additional 3 preparation tests, CLI
typecheck, Biome, and diff checks. Generated-context provenance was
ported into the split preflight fixtures without restoring the obsolete
monolith.
- Exact-head CI for `9a31537785ef2d456901de622721ed215627fdec` passed:
40 checks green, all five required contexts passed, and there were 0
failures, cancellations, or pending checks. The only skips were the
expected docs-only job and two duplicate NVSkills request jobs. This
includes all five CLI shards plus the aggregate, both CodeQL languages,
both sandbox image builds, macOS, WSL, four self-hosted runtime checks,
CodeRabbit, and both review advisors.
- Exact-head live E2E for `9a31537785ef2d456901de622721ed215627fdec`
passed:
[`mcp-bridge`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844701),
[`mcp-bridge-dev`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844719),
and
[`ubuntu-repo-cloud-langchain-deepagents-code`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844639).
Stable and dev each passed OpenClaw, Hermes, and DCode 3/3;
authenticated MCP calls passed initially and after restart, credential
rotation, and rebuild, then removal denied access with no provider,
policy, tunnel, or credential residue. The dedicated DCode lane passed
Landlock 5/5, Python egress 14/14, headless inference 10/10, secret
boundary 8/8, Tavily 6/6, and TUI 4/4; BuildKit accepted the merged
generated-context handoff, and invalid-credential rebuild failure
remained pre-destructive with the original sandbox, marker, and route
recovered. Artifact inspection found one unchanged pre-existing harness
defect: two OpenShell audit-log filtering subassertions can false-pass
because awk treats `close` as reserved; runtime-output, sandbox-log,
env-file immutability, and raw-secret checks passed, and this PR does
not modify that E2E file.
- The base `test-cli` pre-commit invocation remains affected by Vitest
4.1.9 collapsing repeated `--coverage.exclude` arguments to a
zero-file/invalid summary. All other commit and push hooks passed;
targeted tests and the authoritative sharded CI coverage checks provide
the exact-head gate.
- `npm run docs` completed with 0 errors and 2 pre-existing Fern
warnings. Two documentation-writer audits confirmed the final behavior
is accurately documented.

## Type of Change

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

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent security
and correctness reviews passed after fixes; destructive-boundary
rollback, capability migration, the process-local integrity-bound
snapshot handoff (sealed memfd preferred, anonymous `O_TMPFILE`
fallback), cross-language secret-pattern parity, policy fidelity, and
the #6224 boundary were checked.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [x] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced managed MCP bridge support with managed-only configuration
snapshots for safer add/restart/rebuild/teardown.
* Network policy protocol rules now support protocol-specific matching
plus stricter `endpoint.path` validation.
* **Bug Fixes**
* Stronger fail-fast validation for MCP server names/hostnames and
endpoint details (rejected before changes are applied).
* Rebuild flows improved to preserve/replay custom policies and validate
after MCP preparation, with rollback on failure.
* **Documentation**
* Updated setup/quickstart/reference and MCP bridge/rebuild guidance for
managed MCP capability v2 behavior and stricter validation rules.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the v0.0.74 label Jul 4, 2026
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
<!-- 1-3 sentences: what this PR does and why. -->

Separates create-time onboarding intent resolution from effectful
materialization while preserving the existing `prepareSandboxCreatePlan`
entry point and runtime behavior. This establishes a typed internal seam
for later FSM work without persisting temporary paths, cleanup
callbacks, or messaging credential values.

## Changes
<!-- Bullet list of key changes. -->

- Add a deterministic `SandboxCreateIntent` resolver with explicit
credential metadata, policy inputs, GPU arguments, and provider
contributions.
- Materialize temporary policy files, resource flags, provider
cleanup/upserts, and concrete create arguments in a separate effectful
phase.
- Keep the serializable intent contract in a dedicated type-only module
so the execution path remains focused.
- Reject changed credential availability or provider type before any
materialization effects run.
- Preserve provider ordering, channel filtering, policy-tier behavior,
and the existing compatibility wrapper.
- Add characterization coverage for serialization, credential-value
exclusion, effect ordering, stale bindings, disabled channels, GPU
behavior, and provider deduplication.
- Keep FSM/session capture out of scope: recreate still reaches this
seam after the existing destructive boundary, which requires a separate
migration.
- Local verification used Node.js 22: 39 focused tests pass, CLI
typecheck/build and repository checks pass, and GitHub verifies all
three signed commits. The repository-wide `test-cli` hook was attempted,
then skipped for the commits because macOS lacks GNU `script -qec` and
Docker; a separate CLI run reached 5,335 passing tests with 18 unrelated
timeout failures in untouched files.

## Type of Change

- [x] 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
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: internal behavior-preserving
refactor with no CLI, prompt, output, persistence, event, or public API
change.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: reviewed
credential-value exclusion, binding validation, and side-effect
ordering; focused tests assert no effects run for stale bindings.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [ ] 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Strengthened validation during sandbox creation so mismatched or
missing messaging credential bindings are detected early, preventing
partial side effects.
* Improved determinism and correctness of messaging provider request
generation and the resulting provider wiring order.

* **Refactor**
* Reworked sandbox creation into a two-stage flow: deriving a
serializable “intent” and then materializing it into final creation
arguments and providers.
* Updated messaging provider and active channel resolution to be
request-driven, with consistent ordering and Hermes gateway integration.

* **Tests**
* Expanded and added coverage for intent resolution, credential-binding
failure behavior, deterministic output, and effect ordering.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
<!-- 1-3 sentences: what this PR does and why. -->

Prevents a Ready LangChain Deep Agents Code sandbox from being deleted
until its recorded OpenShell gateway, live inference route, managed base
image, and staged build context have been validated and sealed. A
DCode-only, process-local orchestration facade carries that verified
gateway/context pair through recreation, fixing the post-delete
port-8080 false conflict from NVIDIA#6195 without adding generalized persisted
replay or FSM state.

## Related Issue
<!-- Fixes #NNN or Closes #NNN. Remove this section if none. -->

Fixes NVIDIA#6195
Refs NVIDIA#6218
Refs NVIDIA#6224
Refs NVIDIA#6226
Refs NVIDIA#5801

## Changes
<!-- Bullet list of key changes. -->

- Bind DCode rebuilds to the sandbox's recorded gateway before
credential validation, probe the live `https://inference.local` route
without exposing credentials, and revalidate the gateway, registry
entry, route, schema, base image, and staged context immediately before
deletion.
- Build a disposable DCode image and fingerprint its managed build
context before mutation, then pass the retained context and recorded
gateway as a one-shot handoff through the create-intent seam landed in
NVIDIA#6218. Ordinary onboarding and non-DCode rebuild behavior remain
unchanged.
- Put the DCode lifecycle in `rebuild-dcode-orchestrator.ts`: it owns
scoped gateway cleanup, target preflight, replacement preparation,
mutation-edge revalidation, recovery behavior, and one-shot handoff
cleanup while depending on injected generic rebuild callbacks. This
reduced `rebuild.ts` from 1,599 to 1,460 lines and split its 1,477-line
flow spec into focused generic and DCode suites plus a shared harness.
- Extract the onboarding handoff into `prepared-dcode-rebuild.ts`, which
owns gateway validation, ordinary-versus-prepared staging/patching, and
consume-before-call semantics. The top-level `onboard.ts` entrypoint is
four lines smaller than `main`.
- Preserve the existing sandbox when initial preflight fails; if
mutation-edge validation fails after backup, keep the harmless backup
but do not delete or recreate the sandbox. Prepared-backup recovery
skips unavailable live-route probes and does not take a second backup.
Messaging-preflight aborts also restore the prior process gateway.
- Harden managed-context sealing against symlink substitution, pathname
replacement, and in-place mutation by reading regular files through
`O_NOFOLLOW` file descriptors and checking identity/metadata before and
after each read.
- Add a live DCode lifecycle fixture that rotates the credential stored
in the real gateway, waits for a 401, runs rebuild without a host
credential, and proves there was no backup/delete/create while container
identity, readiness, and a workspace marker remain intact.
- Document the DCode rebuild preflight and backup boundary in the
quickstart. A documentation review confirmed no further user-facing
changes are needed.
- Keep generalized FSM/session capture and cross-agent replay out of
scope for NVIDIA#6224/NVIDIA#6226; true build/health/swap replacement remains NVIDIA#5801.

## Type of Change

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent final
architecture and nine-category security reviews both passed the narrowed
DCode-only design with no blocker; the security review included the
final orchestration extraction and managed-context TOCTOU hardening.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [x] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

Commands and evidence:

- `make check` — passed on the final narrowed implementation under the
repository-expected `umask 022`.
- `npm test` — 998 files passed, 2 platform suites skipped; 11,277 tests
passed, 35 skipped.
- Post-merge focused/current-main set — 96/96 passed across generic
rebuild flow, DCode flow, managed-image preflight, prepared handoff,
inference-provider compatibility, and messaging setup; the final
descriptor-pinning, target-guard, and fail-closed review deltas passed
27/27 focused cases.
- Additional affected environment-failure audit — 121/121 passed; E2E
support — 26/26 passed.
- `npm run build:cli` and `npm run typecheck:cli` — passed.
- Current-tree Vitest project-membership check — 1,075 files across
eight disjoint projects; source-shape, test-file-size, and changed-test
conditional budgets passed.
- `npm run docs` — passed with 0 errors and 2 pre-existing warnings.
- Pre-push plugin and CLI TypeScript gates — passed.
- GitHub commit verification — 18/18 commits report `verified=true`,
reason `valid`.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added support for a new DCode rebuild flow during onboarding and
sandbox recreation.
* Introduced a preflight check for inference routing before rebuilds
proceed.

* **Bug Fixes**
* Rebuilds now fail safely when credentials or routing are invalid,
helping prevent unsafe sandbox changes.
* Improved rebuild handling so existing sandboxes and backups are
preserved if a later check fails.

* **Documentation**
* Updated the quickstart guide with clearer rebuild and backup behavior
for managed DCode sandboxes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

Hardens the managed-MCP work merged in NVIDIA#5876 so DCode rebuilds validate
every reconstructable input before crossing the destructive delete
boundary, preserve exact policy intent, and migrate legacy managed MCP
state fail-closed. Prepared rebuild artifacts and the derived MCP
runtime snapshot remain ephemeral and process-local; neither is
persisted in FSM or checkpoint state, so this does not implement NVIDIA#6224.

## Related Issue

Refs NVIDIA#5876
Refs NVIDIA#6195
Refs NVIDIA#6218

## Changes

- Revalidate DCode route, image, Dockerfile, reasoning, web-search, and
MCP inputs after preparation and before NIM stop or sandbox deletion;
restore MCP state and relock shields on failure.
- Preserve exact custom network policy replay while keeping generated
MCP rules under the MCP adapter's exclusive ownership.
- Add protocol-specific policy schema validation for REST, WebSocket,
JSON-RPC, and MCP matchers, including cross-rule `tools/call` conflict
rejection.
- Pin Deep Agents Code 0.1.30 and load only a strict, canonicalized
managed MCP projection from a process-local integrity-bound snapshot.
Sealed memfd is preferred; when OpenShell seccomp blocks it, an
anonymous `O_TMPFILE` inode is reopened read-only and bound by
descriptor, device, inode, size, kind, and SHA-256, with ambient
discovery disabled.
- Bind the canonical TypeScript secret-pattern source and flags to one
shared behavior corpus executed through the Bash and Python DCode
enforcement boundaries, including the full ECMAScript whitespace set.
- Add capability-v2 gating and legacy-v1 teardown/rollback that
preserves unrelated user configuration and fails closed on malformed,
unsafe, or drifted state.
- Add rebuild, migration, runtime-patch, schema, snapshot, and lifecycle
coverage; update the MCP, policy, security, command, and DCode
documentation.

Verification notes:

- Final DCode-adjacent run: 9 files, 187 tests passed; the focused
descriptor/projection run passed 4 files and 138 tests.
- Final review-follow-up run: 82 focused Bash/Python/TypeScript parity
and descriptor-fallback tests passed, including all 25 ECMAScript
whitespace code points under both `C` and `C.UTF-8` Bash locales.
- Full pre-squash-equivalent run: 1,068 files passed, 2 skipped; 12,149
tests passed, 35 skipped.
- CLI coverage ratchet passed with the repository include/exclude set
expressed as one Vitest glob: lines 65.24%, statements 64.45%, functions
67.06%, branches 57.21%.
- Python compile, Biome, ShellCheck, shfmt, source-shape, test-size,
repository, secret-scan, and diff checks passed. The normal push hook
passed CLI typechecking.
- Main-sync validation after merging NVIDIA#6265 passed: 9 CLI files/82 tests,
6 integration files/174 tests, an additional 3 preparation tests, CLI
typecheck, Biome, and diff checks. Generated-context provenance was
ported into the split preflight fixtures without restoring the obsolete
monolith.
- Exact-head CI for `9a31537785ef2d456901de622721ed215627fdec` passed:
40 checks green, all five required contexts passed, and there were 0
failures, cancellations, or pending checks. The only skips were the
expected docs-only job and two duplicate NVSkills request jobs. This
includes all five CLI shards plus the aggregate, both CodeQL languages,
both sandbox image builds, macOS, WSL, four self-hosted runtime checks,
CodeRabbit, and both review advisors.
- Exact-head live E2E for `9a31537785ef2d456901de622721ed215627fdec`
passed:
[`mcp-bridge`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844701),
[`mcp-bridge-dev`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844719),
and
[`ubuntu-repo-cloud-langchain-deepagents-code`](https://github.com/NVIDIA/NemoClaw/actions/runs/28696844639).
Stable and dev each passed OpenClaw, Hermes, and DCode 3/3;
authenticated MCP calls passed initially and after restart, credential
rotation, and rebuild, then removal denied access with no provider,
policy, tunnel, or credential residue. The dedicated DCode lane passed
Landlock 5/5, Python egress 14/14, headless inference 10/10, secret
boundary 8/8, Tavily 6/6, and TUI 4/4; BuildKit accepted the merged
generated-context handoff, and invalid-credential rebuild failure
remained pre-destructive with the original sandbox, marker, and route
recovered. Artifact inspection found one unchanged pre-existing harness
defect: two OpenShell audit-log filtering subassertions can false-pass
because awk treats `close` as reserved; runtime-output, sandbox-log,
env-file immutability, and raw-secret checks passed, and this PR does
not modify that E2E file.
- The base `test-cli` pre-commit invocation remains affected by Vitest
4.1.9 collapsing repeated `--coverage.exclude` arguments to a
zero-file/invalid summary. All other commit and push hooks passed;
targeted tests and the authoritative sharded CI coverage checks provide
the exact-head gate.
- `npm run docs` completed with 0 errors and 2 pre-existing Fern
warnings. Two documentation-writer audits confirmed the final behavior
is accurately documented.

## Type of Change

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

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent security
and correctness reviews passed after fixes; destructive-boundary
rollback, capability migration, the process-local integrity-bound
snapshot handoff (sealed memfd preferred, anonymous `O_TMPFILE`
fallback), cross-language secret-pattern parity, policy fidelity, and
the NVIDIA#6224 boundary were checked.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [x] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced managed MCP bridge support with managed-only configuration
snapshots for safer add/restart/rebuild/teardown.
* Network policy protocol rules now support protocol-specific matching
plus stricter `endpoint.path` validation.
* **Bug Fixes**
* Stronger fail-fast validation for MCP server names/hostnames and
endpoint details (rejected before changes are applied).
* Rebuild flows improved to preserve/replay custom policies and validate
after MCP preparation, with rollback on failure.
* **Documentation**
* Updated setup/quickstart/reference and MCP bridge/rebuild guidance for
managed MCP capability v2 behavior and stricter validation rules.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants