feat(onboard): declare runtime host mount support - #8368
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
… into feat/read-only-host-mounts-8274
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
area: onboarding
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (31)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (27)
📝 WalkthroughWalkthroughThe change adds runtime-provider capability checks for read-only host mounts, validates them during onboarding and session preparation, simplifies sandbox state handling, removes obsolete gateway and remote-access behavior, and updates related tests and documentation. ChangesSandbox runtime and state changes
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to Portable host-mount requests may receive a generic Docker-driver error instead of the documented provider-specific diagnostic. The request is still rejected safely, so the PR is mergeable with owner awareness or follow-up to make the delegated validator the sole failure path. Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardCommand
participant SessionBootstrap
participant RuntimeProviderRegistry
participant RuntimeProvider
User->>OnboardCommand: start onboarding with host mounts
OnboardCommand->>SessionBootstrap: prepare fresh or resumed session
SessionBootstrap->>RuntimeProviderRegistry: resolve provider capability
RuntimeProviderRegistry->>RuntimeProvider: check readOnlyHostMounts and host platform
RuntimeProvider-->>SessionBootstrap: capability or selection error
SessionBootstrap-->>OnboardCommand: create or reject session
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit fa0020a in the TypeScript / code-coverage/cliThe overall coverage in commit fa0020a in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed current head 29d0ab3a3e24e306a47f20102d6a109b0d3db6c9.
This PR creates a user-visible runtime-provider host-mount capability contract and canonical support documentation across Docker, Podman, and OpenShell MXC, but it has no linked accepted issue or design decision defining ownership, lifecycle, compatibility, security, and qualification expectations. The repository product-scope gate blocks that supported surface until maintainers establish those decisions.
The effective diff is also stale against current main: it removes unrelated current sandbox-status SSH guidance and associated safety tests while introducing the host-mount changes. Refresh without reverting current behavior. Then either scope the change to an accepted current consumer or record the product decision, and rerun security, behavior, documentation, and required CI review on the resulting exact head.
senthilr-nv
left a comment
senthilr-nv
left a comment
There was a problem hiding this comment.
Review of commit 29d0ab3a3e24e306a47f20102d6a109b0d3db6c9: this PR is not approval-ready.
Although the linked issue was accepted, the implementation is stale against current onboarding and gateway behavior. Five required checks are missing, and CodeRabbit still has an unresolved major security finding in src/lib/onboard/docker-driver-gateway-config.ts. The branch also moved relative to main during gate evaluation.
Reconstruct the accepted host-mount behavior from current main, resolve the security finding, add regression evidence for the current gateway lifecycle, and rerun every required gate. Do not mechanically refresh this branch because the surrounding behavior has changed.
|
🌿 Preview your docs: https://nvidia-preview-pr-8368.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Later signed commits address the findings from this earlier revision. All review threads are resolved, and CI passes for commit fa0020a.
5 participants
Summary
Declare read-only host-mount support through the selected runtime-provider contract and reject unsupported providers or host platforms before onboarding mutates durable session state. Docker remains supported on Linux and WSL, while Kubernetes, Podman, and OpenShell MXC fail closed with provider-specific reasons. This builds on merged #8280.
Related Issue
Fixes #8343. Builds on merged #8280.
Changes
Type of Change
Quality Gates
ff20cc572; no findings remain.Security Review
Documentation Writer Review
docs-updateddocs/manage-sandboxes/workspace-files.mdxdocuments the Docker, Kubernetes, Podman, and OpenShell MXC read-only host-mount support matrix, fail-closed provider and host-platform checks, Docker fallback prohibition, and qualification requirements for future runtime-provider implementations in the OpenClaw, Hermes, and LangChain Deep Agents Code guide variants. Independent review of commitfa0020af0confirmed that the PR-owned patch is unchanged after mergingmainat68323e2ef, the documentation matches the runtime-provider declarations and enforcement paths, and requested or persisted unsupported mounts are rejected before session or runtime mutation. Recorded validation includes 251 earlier tests passing, the final locked-runtime regression passing 2/2,npm run build:cli,npm run typecheck:cli, andnpm run test-conditionals:scan -- --top 25passing.npm run docspassed with 0 errors and 2 existing warnings. Ten other test invocations did not start because the isolated clone lacked generated shared and catalog artifacts; GitHub CI remains required.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every pushed commit appears asVerifiedin GitHubnpm run validate:prcompleted applicable checks, but the local dependency tree could not import the/oxfmtpackage required byoxfmt.config.ts; GitHub CI remains requirednpm run build:cliandnpm run typecheck:clipassednpm run test-conditionals:scan -- --top 25passednpm run docsbuilds without warnings (doc changes only) — 0 errors; 2 pre-existing hidden-page warnings remainSigned-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Bug Fixes