Skip to content

fix(vllm): guard managed downloads by storage capacity - #6771

Merged
cv merged 21 commits into
mainfrom
fix/6757-vllm-storage-preflight
Jul 13, 2026
Merged

fix(vllm): guard managed downloads by storage capacity#6771
cv merged 21 commits into
mainfrom
fix/6757-vllm-storage-preflight

Conversation

@sandl99

@sandl99 sandl99 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Managed vLLM onboarding now checks Docker image storage and the Hugging Face model cache before starting large downloads, and stops safely when capacity is insufficient or cannot be verified unless the operator explicitly accepts the warning.
The preflight fails closed when Docker daemon and client filesystem identity is ambiguous, including nested PID or mount namespaces, before either an uncached image pull or model download can start.
Platform-specific immutable NGC digests and consistent Docker client settings make cached-image detection authoritative and prevent implicit pulls from bypassing the checks.

Related Issue

Refs #6757

Implementation is complete; keep the issue open until bounded-Docker and physical DGX Station acceptance are recorded.

Changes

  • Add a storage probe consumed by managed installVllm that resolves classic Docker and containerd image/pull-staging filesystems plus the Hugging Face cache, and fails closed when capacity cannot be established.
  • Verify the selected canonical Docker Unix socket peer with Linux SO_PEERCRED, require the daemon peer PID to be visible, and compare stable client/daemon mount tables before trusting client-side statfs; nested PID/mount namespaces and unverifiable endpoints require the documented explicit override.
  • Gate uncached image pulls and model downloads, recheck model capacity after the image pull, and verify model-cache bind identity with a bounded daemon-side sentinel before starting a large download.
  • Pin managed images to architecture-specific NGC digests, carry exact compressed image-layer sizes plus published model sizes, reuse exact cached digests safely, and use --pull=never for model and serving containers.
  • Preserve Docker client selection and TLS settings across storage/cache probes, pulls, container launch, and lifecycle checks.
  • Update managed-vLLM documentation, command reference, platform metadata, and fail-closed regression coverage.

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:
  • 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: Exact-head GPT advisor, CodeRabbit, and CodeQL are clean; @cjagwani re-review is requested on f2482589 after addressing the prior P1.
  • 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
  • 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 or justification: 76 focused tests across 2 files and 196 merge-base affected tests across 13 files passed; CLI typecheck, Biome, npm run check:diff, and npm run docs:strict passed with 0 errors and 2 pre-existing Fern warnings.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Exact-head hosted CI passed all applicable shards, with 45 successful and 5 expected skipped checks overall.
  • 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) — build passed with 0 errors; Fern reported 2 pre-existing warnings.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Exact-head review and live validation:

  • GPT PR advisor: merge_as_is, 0 required findings, 0 warnings, and 0 optional suggestions.
  • Nemotron PR advisor: merge_as_is; its sole warning about smol-toml is inapplicable because package.json and the lockfile already pin exact version 1.7.0.
  • CodeRabbit and JavaScript/Python CodeQL: successful with no unresolved review threads; the prior test-only filesystem-race alert is fixed on this head.
  • Trusted E2E run: cloud-onboard, inference-routing, network-policy, onboard-repair, and onboard-resume all passed; the controller verified the evidence and closed the exact-head check successfully.

Bounded-Docker and physical DGX Station acceptance remain required before #6757 closes.


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

Summary by CodeRabbit

  • New Features
    • Managed vLLM now uses immutable digest-pinned images (by architecture) and pins host profiles to immutable NGC platform digests.
    • Added managed vLLM disk-capacity preflight checks (Docker image storage + model cache), including a recheck after the image pull.
    • Containers now use --restart unless-stopped for managed runs.
  • Bug Fixes
    • Improved managed vLLM onboarding robustness: standardized Docker env handling, safer token forwarding, and consistent --pull=never behavior.
  • Documentation
    • Updated provider support and vLLM setup/reference to include digest/image-size mappings and onboarding/storage-warning flag behavior (NEMOCLAW_YES vs NEMOCLAW_IGNORE_VLLM_DISK_SPACE).
  • Tests
    • Expanded vLLM unit coverage for storage probing and digest-based profile detection.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 added provider: vllm vLLM local or hosted provider behavior area: docs Documentation, examples, guides, or docs build area: local-models Local model providers, downloads, launch, or connectivity bug-fix PR fixes a bug or regression platform: container Affects Docker, containerd, Podman, or images platform: dgx-station Affects DGX Station hardware or workflows labels Jul 13, 2026
@sandl99 sandl99 self-assigned this Jul 13, 2026
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Managed vLLM now selects architecture-specific immutable NGC image digests, records image and model download sizes, probes Docker and model-cache capacity before downloads, sanitizes Docker environments, updates onboarding detection, and documents storage warnings and overrides.

Changes

Managed vLLM installation

Layer / File(s) Summary
Storage and download-size foundation
src/lib/inference/vllm-models.ts, src/lib/inference/vllm-storage.ts, src/lib/inference/*test.ts
Adds model download sizes, storage contracts, Docker storage detection, model-cache probing, and tests for capacity calculations and detection paths.
Digest-pinned installation and storage gates
src/lib/inference/vllm.ts, src/lib/inference/vllm-docker-env.ts, src/lib/inference/*test.ts, test/detect-vllm-profile.test.ts
Selects architecture-specific digest images, prevents implicit pulls, forwards sanitized Docker environments, and gates image/model downloads on storage checks.
Onboarding Docker detection
src/lib/onboard/provider-host-state.ts, src/lib/onboard/provider-host-state.test.ts
Inspects the pinned image with propagated Docker variables, timeout handling, and ignored errors.
Digest and storage documentation
ci/platform-matrix.json, docs/inference/*.mdx, docs/reference/*.mdx
Documents host-specific digests, image sizes, storage checks, override behavior, and confirmation semantics.

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

Sequence Diagram(s)

sequenceDiagram
  participant Onboarding
  participant InstallVllm
  participant Storage
  participant Docker
  participant ModelCache
  Onboarding->>Docker: inspect pinned vLLM image
  InstallVllm->>Storage: probe Docker capacity
  Storage-->>InstallVllm: return capacity result
  InstallVllm->>Docker: pull pinned image
  InstallVllm->>ModelCache: probe model-cache capacity
  InstallVllm->>Docker: download model and start container
Loading

Possibly related issues

Possibly related PRs

Suggested labels: area: inference

Suggested reviewers: miyoungc, laitingsheng, cv, yimoj, apurvvkumaria

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.89% 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
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 clearly matches the main change: adding storage-capacity guards for managed vLLM downloads.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/6757-vllm-storage-preflight

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

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Optional E2E: None

Dispatch hint: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard: Required validation floor: platform/install changes must complete clean-host onboarding with the pinned runtime dependencies.
  • inference-routing: Required validation floor: managed-vLLM provider selection and host-to-sandbox inference routing changed.
  • network-policy: Required validation floor: local vLLM reachability remains constrained by the intended sandbox network-policy boundary.
  • onboard-repair: Required validation floor: provider-host state changes can affect recovery convergence after interrupted onboarding.
  • onboard-resume: Required validation floor: provider-host state changes can affect resumed onboarding's persisted-state and host-state reconciliation.

Optional E2E

  • None.

New E2E recommendations

  • managed-vllm-onboarding (high): No existing live E2E found that exercises a managed-vLLM image digest pull, storage-gate behavior, model download, readiness probe, and an assistant request on a physical supported GPU host. Unit tests cannot validate Docker daemon locality, bind identity, GPU runtime startup, or the actual inference route.
    • Suggested test: Add a physical-DGX-Spark managed-vLLM onboarding E2E that validates the pinned arm64 digest, storage preflight/explicit override behavior, vLLM readiness, sandbox inference.local completion, and cleanup.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

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/6757-vllm-storag... branch remains at 79%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 986b883 fix/6757-vllm-storag... db8655a +/-
src/lib/adapter...shell/client.ts 88% 83% -5%
src/lib/messagi...n-validation.ts 100% 96% -4%
src/lib/messagi.../persistence.ts 89% 86% -3%
src/lib/inference/vllm.ts 74% 77% +3%
src/lib/state/config-io.ts 91% 94% +3%
src/lib/runner.ts 72% 75% +3%
src/lib/inferen.../vllm-models.ts 66% 72% +6%
src/lib/agent/dashboard-ui.ts 83% 89% +6%
src/lib/inferen...vllm-storage.ts 0% 90% +90%
src/lib/inferen...m-docker-env.ts 0% 100% +100%

Updated July 13, 2026 22:43 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Primary next action: No advisor follow-up required beyond maintainer review.
Findings: 0 blockers · 0 warnings · 0 optional suggestions
Status: No actionable findings remain in the canonical review ledger.
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

E2E guidance

Advisory only: coverage and selector recommendations are non-authoritative. E2E / PR Gate independently computes and dispatches trusted jobs without consuming this output.

Recommended coverage: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Recommended selectors: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume

  • cloud-onboard — Selected from the trusted checked-in E2E coverage inventory.

  • inference-routing — Selected from the trusted checked-in E2E coverage inventory.

  • network-policy — Selected from the trusted checked-in E2E coverage inventory.

  • onboard-repair — Selected from the trusted checked-in E2E coverage inventory.

  • onboard-resume — Selected from the trusted checked-in E2E coverage inventory.

  • cloud-onboard — Selected as a trusted checked-in E2E job.

  • inference-routing — Selected as a trusted checked-in E2E job.

  • network-policy — Selected as a trusted checked-in E2E job.

  • onboard-repair — Selected as a trusted checked-in E2E job.

  • onboard-resume — Selected as a trusted checked-in E2E job.

2 optional coverage items · 2 optional selectors · 0 new-test recommendations
  • Optional coverage gpu-e2e — Selected from the trusted checked-in E2E coverage inventory.
  • Optional coverage spark-install — Selected from the trusted checked-in E2E coverage inventory.
  • Optional selector gpu-e2e — Selected as a trusted checked-in E2E job.
  • Optional selector spark-install — Selected as a trusted checked-in E2E job.

Workflow run details

This is an automated, non-authoritative review. Findings are inputs to maintainer adjudication. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision.

@sandl99 sandl99 added the VDR Linked to VDR finding label Jul 13, 2026

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

Actionable comments posted: 1

🤖 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/inference/vllm.test.ts`:
- Around line 155-162: Replace the arch-dependent ternary expectations with
tests that stub process.arch to both arm64 and x64 and assert fixed digest and
imageDownloadSizeBytes literals for each branch in
src/lib/inference/vllm.test.ts lines 155-162. Apply the same per-branch
fixed-literal image assertion in test/detect-vllm-profile.test.ts lines 45-49,
ensuring both sites exercise the production architecture mapping rather than
mirroring its conditional.
🪄 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: 9cb20284-3c15-4269-b2d1-8f4246bd424a

📥 Commits

Reviewing files that changed from the base of the PR and between 3de1de6 and 07503c4.

📒 Files selected for processing (16)
  • ci/platform-matrix.json
  • docs/inference/choose-inference-provider.mdx
  • docs/inference/set-up-vllm.mdx
  • docs/reference/commands.mdx
  • docs/reference/platform-support.mdx
  • src/lib/inference/vllm-docker-env.test.ts
  • src/lib/inference/vllm-docker-env.ts
  • src/lib/inference/vllm-models.test.ts
  • src/lib/inference/vllm-models.ts
  • src/lib/inference/vllm-storage.test.ts
  • src/lib/inference/vllm-storage.ts
  • src/lib/inference/vllm.test.ts
  • src/lib/inference/vllm.ts
  • src/lib/onboard/provider-host-state.test.ts
  • src/lib/onboard/provider-host-state.ts
  • test/detect-vllm-profile.test.ts

Comment thread src/lib/inference/vllm.test.ts Outdated
sandl99 added 3 commits July 13, 2026 22:05
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@prekshivyas prekshivyas self-assigned this Jul 13, 2026
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

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

🧹 Nitpick comments (1)
docs/inference/set-up-vllm.mdx (1)

71-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Digest/size block has no drift guard.

The three sha256 digests and compressed sizes here (verified correct against VLLM_IMAGES in src/lib/inference/vllm.ts) are hand-maintained on this page, unlike the platform-matrix-derived tables in docs/reference/platform-support.mdx and docs/inference/choose-inference-provider.mdx, which are protected by the platform-matrix-sync pre-commit hook and scripts/generate-platform-docs.py --check. When a new NGC release is pinned in vllm.ts, this block can silently go stale.

Consider either generating this block from the same source (e.g. via ci/platform-matrix.json/the generator script) or adding a small CI check that diffs these hashes/sizes against VLLM_IMAGES.

🤖 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 `@docs/inference/set-up-vllm.mdx` around lines 71 - 82, Protect the
managed-profile digest and compressed-size block in the vLLM documentation from
drifting from the VLLM_IMAGES definitions in src/lib/inference/vllm.ts. Extend
the existing platform-doc generation/check flow or add a focused CI validation
that derives the three image entries and verifies their digests and sizes,
including the referenced image tags, against this block, failing when they
differ.
🤖 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.

Nitpick comments:
In `@docs/inference/set-up-vllm.mdx`:
- Around line 71-82: Protect the managed-profile digest and compressed-size
block in the vLLM documentation from drifting from the VLLM_IMAGES definitions
in src/lib/inference/vllm.ts. Extend the existing platform-doc generation/check
flow or add a focused CI validation that derives the three image entries and
verifies their digests and sizes, including the referenced image tags, against
this block, failing when they differ.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2b305174-6ba9-4cd2-8405-9a6fa4737ca5

📥 Commits

Reviewing files that changed from the base of the PR and between 3de1de6 and e52905f.

📒 Files selected for processing (16)
  • ci/platform-matrix.json
  • docs/inference/choose-inference-provider.mdx
  • docs/inference/set-up-vllm.mdx
  • docs/reference/commands.mdx
  • docs/reference/platform-support.mdx
  • src/lib/inference/vllm-docker-env.test.ts
  • src/lib/inference/vllm-docker-env.ts
  • src/lib/inference/vllm-models.test.ts
  • src/lib/inference/vllm-models.ts
  • src/lib/inference/vllm-storage.test.ts
  • src/lib/inference/vllm-storage.ts
  • src/lib/inference/vllm.test.ts
  • src/lib/inference/vllm.ts
  • src/lib/onboard/provider-host-state.test.ts
  • src/lib/onboard/provider-host-state.ts
  • test/detect-vllm-profile.test.ts

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

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

Actionable comments posted: 3

🤖 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 `@ci/platform-matrix.json`:
- Line 150: Update ci/platform-matrix.json at lines 150-150 to state that DGX
Station is deferred and not validated end-to-end; regenerate the corresponding
provider row in docs/inference/choose-inference-provider.mdx at lines 35-35 and
docs/reference/platform-support.mdx at lines 107-107, and add the same caveat to
the digest bullets in docs/inference/set-up-vllm.mdx at lines 71-75.

In `@src/lib/inference/vllm-storage.ts`:
- Around line 183-198: Update the endpoint selection in the Docker context
validation flow so dockerHost takes precedence over an explicit dockerContext,
including the “default” context. Remove the conditional that clears the endpoint
when explicitContext is set, and preserve the existing remote-endpoint rejection
for non-local endpoints; update the related test expectation so the DOCKER_HOST
remote-daemon case fails closed.

In `@src/lib/onboard/provider-host-state.ts`:
- Around line 173-179: Use the caller-provided environment as the authoritative
Docker environment: update buildVllmDockerEnv in
src/lib/onboard/provider-host-state.ts at lines 173-179 to pass input.env, and
update src/lib/onboard/provider-host-state.test.ts at lines 101-102 to provide
DOCKER_CONTEXT and DOCKER_HOST through detectInferenceProviderHostState(...).env
rather than stubbing process.env.
🪄 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: f9034720-b4a5-4c2a-80a6-1e52b137d005

📥 Commits

Reviewing files that changed from the base of the PR and between 3de1de6 and cab3598.

📒 Files selected for processing (17)
  • ci/platform-matrix.json
  • docs/inference/choose-inference-provider.mdx
  • docs/inference/set-up-vllm.mdx
  • docs/reference/commands.mdx
  • docs/reference/platform-support.mdx
  • src/lib/inference/vllm-docker-env.test.ts
  • src/lib/inference/vllm-docker-env.ts
  • src/lib/inference/vllm-models.test.ts
  • src/lib/inference/vllm-models.ts
  • src/lib/inference/vllm-storage.test.ts
  • src/lib/inference/vllm-storage.ts
  • src/lib/inference/vllm.test.ts
  • src/lib/inference/vllm.ts
  • src/lib/onboard/provider-host-state.test.ts
  • src/lib/onboard/provider-host-state.ts
  • test/detect-vllm-profile.test.ts
  • test/inference-options-docs.test.ts

Comment thread ci/platform-matrix.json Outdated
Comment thread src/lib/inference/vllm-storage.ts Outdated
Comment thread src/lib/onboard/provider-host-state.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] The canonical socket pathname still does not prove that client-side capacity and daemon-side bind mounts name the same filesystem.

On exact head b9c4f34, nativeDockerHostProblem accepts an unset endpoint or /var/run/docker.sock / unix:///var/run/docker.sock after checking only client platform/os.release and daemon docker-info strings at src/lib/inference/vllm-storage.ts:179-208. In a Linux client container with the host socket bind-mounted at that canonical path, all of those checks report native Linux and default context. modelStorageAccepted then stats os.homedir()/.cache/huggingface in the client container namespace at src/lib/inference/vllm.ts:127-133 and 679-701, while the later docker run -v of that path is resolved in the daemon host namespace.

This is a concrete repository topology, not just a renamed proxy: test/e2e/Dockerfile.full-e2e installs only the Docker CLI, explicitly expects the host socket at runtime, and runs full install/onboard inside that client container. Docker documents that bind sources are resolved on the daemon host and that -v creates a missing source there. With a cached image, NemoClaw can therefore see ample client-overlay capacity, skip the warning, then start a large model download onto a small or different daemon-host filesystem.

This is an availability/correctness merge blocker, not a standalone security exploit. The noncanonical-socket rejection added here is correct, but endpoint allowlisting alone cannot establish bind identity. Please either prove daemon-visible bind identity/capacity, or fail closed when client-container/mount-namespace execution makes that identity ambiguous and require the existing explicit override. Add a cached-image regression for a canonical host socket bind-mounted into a client container.

References: https://docs.docker.com/engine/storage/bind-mounts/ and https://docs.docker.com/reference/cli/docker/container/run/

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Addressed the exact-head client-container P1 in fe02076. The locality probe now fails closed when the Docker CLI is containerized (Docker/Podman/Kubernetes/LXC signals) or does not share the init mount namespace, so mounting the canonical /var/run/docker.sock no longer makes client-side statfs authoritative. The existing NEMOCLAW_IGNORE_VLLM_DISK_SPACE=1 escape hatch remains the explicit operator override.

Coverage now exercises the public resolver with DOCKER_HOST=unix:///var/run/docker.sock plus a /.dockerenv client-container marker, covers a private mount namespace, and carries the same reason through the cached-image install path while asserting no pull, model helper, or long-lived container starts. Validation: 70 focused tests, 190 merge-base changed tests, CLI typecheck, npm run check:diff, and docs:strict (0 errors; 2 pre-existing Fern warnings). Please re-review fe02076.

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes on exact head fe020768db2a58ca53e897f8b93ff2ce20d123d4.

[P1] Do not treat namespace-local PID 1 as proof that the client shares the daemon host mount namespace.

clientSharesInitMountNamespace() compares /proc/self/ns/mnt with /proc/1/ns/mnt (vllm-storage.ts:51-56). In a private PID namespace with its own procfs, /proc/1 is the namespace-local init, not host init. A process created with a private PID and mount namespace can therefore make both links equal even though both are outside the daemon host namespace. With no container marker, an ordinary 0::/ cgroup, the canonical mounted Docker socket, and native-looking Docker info, exact-head probeDockerHostLocality() returns { ok: true }. modelStorageAccepted() can then trust client-side statfs for a daemon-side bind mount and permit the same wrong-filesystem large download.

The ordinary Docker marker case at vllm-storage.test.ts:299-315 is fixed, but the private-mount test at lines 317-330 injects sharesInitMountNamespace: false and never exercises this detector when namespace-local PID 1 yields equal links. Please either prove the cache bind identity from the daemon side with a bounded sentinel round trip, or reliably detect and reject non-initial PID namespaces before treating /proc/1 as host init. Add a nested PID+mount namespace regression with namespace-local procfs and update the documentation claim that every distinct mount namespace is rejected.

All 70 focused exact-head tests pass and no other material issue was found; this topology remains the prior fail-closed storage-boundary blocker.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

@cjagwani I addressed the private-PID-namespace gap in exact head 4482a79e with an authoritative daemon-side bind probe. NemoClaw now creates a mode-0600 random sentinel beneath the nearest existing cache parent and asks the pinned cached vLLM image to read that exact bind using docker run --rm --network=none --pull=never; the token is never passed in argv. A failed or mismatched read fails closed, and the sentinel is removed in finally.

The regression suite now covers the nested PID+mount case where /proc/1 appears local but the daemon sees different storage, including cleanup and proof that the cached path starts no watched pull, model helper, or detached container. Local validation is green: 73 focused tests, 193 merge-base changed tests, CLI typecheck, npm run check:diff, and docs strict (0 errors; 2 existing Fern warnings). Please re-review exact head 4482a79e.

@prekshivyas
prekshivyas requested a review from cjagwani July 13, 2026 19:39
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Follow-up exact head c4681e3c makes the sentinel secrecy guarantee literal: the file path uses an independent random nonce while the content uses a separate secret token, and the regression asserts the bind source path/argv cannot reveal that token. This corrects the implementation detail in my previous note while preserving the bounded daemon-side read and fail-closed behavior. Exact committed-diff gates are green.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Final test-only head a05fae6e tightens the secrecy regression to inspect the secret UUID itself; production behavior is unchanged from c4681e3c. The exact committed-diff gate remains green.

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Exact-head review for a05fae6.

Required before merge (P1): fail closed before an uncached image pull when daemon/client filesystem identity is still ambiguous. In a nested PID+mount namespace with namespace-local procfs, clientSharesInitMountNamespace can still report equality and the canonical socket can pass locality. imageStorageAccepted then accepts client-side statfs for the daemon-reported Docker root (src/lib/inference/vllm.ts:657-676; src/lib/inference/vllm-storage.ts:53-57,360-376) and installVllm starts the image pull at src/lib/inference/vllm.ts:803-815. The new bind sentinel only runs on the model-cache path at src/lib/inference/vllm.ts:686-689 and, for an uncached profile image, not until after that pull at lines 817-823.

The exact-head seam reproduction returned ok with a fabricated 1 TiB client filesystem in this ambiguous topology, so a low-capacity daemon can still receive the roughly 9.6 GB pull before any authoritative bind proof. The 73 focused tests pass, but current coverage exercises bind identity/cached-image rejection, not the uncached no-pull invariant. Please establish authoritative daemon/client filesystem identity (or fail closed) before authorizing the uncached pull, and add a regression test proving dockerPullWithProgressWatchdog is never called for this topology.

Comment thread src/lib/inference/vllm-storage.test.ts Fixed
@cjagwani cjagwani self-assigned this Jul 13, 2026
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

@cjagwani Addressed the exact-head uncached-pull P1 in signed head f2482589. The image-storage preflight no longer trusts namespace-local PID 1: it connects to the selected canonical Unix socket, reads Linux SO_PEERCRED, requires the peer PID to be visible in the client PID namespace, and compares stable client/peer /proc/*/mountinfo snapshots before any Docker-root statfs. A hidden peer (nested PID namespace), unreadable peer, changing snapshot, or different mount table now fails closed before dockerPullWithProgressWatchdog.

I also reproduced the kernel boundary directly on Linux: the canonical host socket mounted into a nested PID namespace reports peer PID 0; with host PID visibility but a private mount namespace, the peer is visible and the mountinfo comparison differs. The new install regression uses an uncached Station image and proves no watched pull, bind probe, model helper, or detached container starts. The new storage regression proves even a fabricated 1 TiB statfs is never called.

The separate CodeQL test-only race is fixed by opening the sentinel once and using fstat plus read on that same fd. Validation: 76 focused tests, 196 merge-base affected tests, CLI typecheck, npm run check:diff, and docs strict (0 errors; 2 existing Fern warnings). Please re-review exact head f2482589.

@prekshivyas
prekshivyas requested a review from cjagwani July 13, 2026 20:06
@prekshivyas

Copy link
Copy Markdown
Collaborator

@cjagwani Final exact-head evidence for f2482589: hosted CI is fully green (45 success, 5 expected skips), GPT advisor reports merge_as_is with zero findings, CodeRabbit and CodeQL are clean with no unresolved threads, and the trusted E2E controller verified all five required targets (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) as passed: https://github.com/NVIDIA/NemoClaw/actions/runs/29281579812. The prior uncached-pull P1 is covered by the hidden-peer no-statfs regression and the install-level no-pull/no-container regression. Please submit the requested exact-head re-review so the stale change-request decision can clear.

@cv
cv enabled auto-merge (squash) July 13, 2026 22:44
@cv
cv merged commit a263901 into main Jul 13, 2026
54 checks passed
@cv
cv deleted the fix/6757-vllm-storage-preflight branch July 13, 2026 22:51
cv added a commit that referenced this pull request Jul 14, 2026
…ness stage (#6782)

<!-- markdownlint-disable MD041 -->
## Summary

The PR Review Advisor's correctness stage now performs purpose-driven
adversarial reasoning when a PR states a correctness guarantee
(fail-closed check, locality invariant, ordering constraint, capacity
gate). Before this change the advisor applied a generic correctness
checklist; it would not systematically enumerate bypass paths or verify
external-system contract assumptions against upstream documentation. A
review of #6771 showed three successive blocker rounds that CI and prior
review missed — all traceable to this gap.

## Changes

- Added one paragraph to the `correctness-state` stage prompt in
`buildPromptTurns()` (`tools/pr-review-advisor/analyze.mts`): when the
PR states a correctness guarantee, the advisor must identify the
guarantee, enumerate specific bypass paths (alternate branches, combined
input states, external-spec assumptions), verify each against the diff,
and check external-system behavior against upstream documentation rather
than internal code consistency alone.

## 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

- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: the change is a prompt
string; correctness is evaluated by running the advisor against real
PRs, not unit tests
- [x] Docs not applicable — justification: no user-facing behavior
change; advisor prompt internals are not documented
- [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: change strengthens the
advisor's security and correctness coverage; no weakening of trust
boundaries

## Verification

- [x] PR description includes the DCO sign-off declaration 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 — command/result or justification:
prompt-only change; no executable test surface
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


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

* **Documentation**
* Enhanced review guidance for assessing correctness guarantees when
changes affect correctness-critical paths, even if not explicitly
labeled.
* Added steps to identify the dependent/claimed guarantee and enumerate
silent bypasses across alternate branches, input state/precedence
differences, and external contract assumptions.
* Clarified how to verify external behavior using repository evidence
and upstream documentation, and to mark ambiguous documentation as
**unverified** rather than confirmed.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
cv pushed a commit that referenced this pull request Jul 14, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Release-prep documentation for v0.0.82 now summarizes user-facing
changes merged since v0.0.81.
It also closes stale wording in the stopped-sandbox backup,
snapshot-clone, Ollama selection, and custom-policy authoring guidance.

## Changes

- Add the `v0.0.82` section to `docs/about/release-notes.mdx` with links
to the focused user guides.
- Document that snapshot clones receive a destination-owned dashboard
port before destructive replacement begins.
- Align `backup-all` guidance with eligible stopped Docker-driver
sandboxes that NemoClaw starts temporarily.
- Describe the running and stopped Ollama menu states without claiming
one fixed label.
- Document runtime rejection of catch-all hosts in custom policy files.

### Source summary

- [#6748](#6748) ->
`docs/about/release-notes.mdx`, `docs/manage-sandboxes/lifecycle.mdx`,
and `docs/reference/commands.mdx`: Summarize non-destructive sandbox
`stop` and `start` commands.
- [#6723](#6723) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/backup-restore.mdx`, and
`docs/reference/commands.mdx`: Record temporary startup and cleanup for
eligible stopped-sandbox backups.
- [#6749](#6749) ->
`docs/about/release-notes.mdx` and
`docs/manage-sandboxes/backup-restore.mdx`: Document destination-owned
dashboard ports for snapshot clones.
- [#6764](#6764) ->
`docs/about/release-notes.mdx`: Summarize installer handling of
route-only onboarding placeholders.
- [#6771](#6771) ->
`docs/about/release-notes.mdx`, `docs/inference/set-up-vllm.mdx`,
`docs/inference/choose-inference-provider.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/platform-support.mdx`: Summarize managed-vLLM storage
gates, immutable image digests, and the explicit override boundary.
- [#6759](#6759) ->
`docs/about/release-notes.mdx`: Record early, actionable OpenShell
gateway-port conflict diagnostics.
- [#6753](#6753) ->
`docs/about/release-notes.mdx` and `docs/inference/set-up-ollama.mdx`:
Document truthful running and stopped Ollama menu states.
- [#6776](#6776) ->
`docs/about/release-notes.mdx`: Summarize proxy-independent loopback
readiness checks.
- [#6769](#6769) ->
`docs/about/release-notes.mdx`: Record compatible endpoint and agent
guidance when Chat Completions is unavailable.
- [#6730](#6730) ->
`docs/about/release-notes.mdx`: Summarize bounded reuse of an eligible
successful Chat Completions check.
- [#6768](#6768) ->
`docs/about/release-notes.mdx`: Record route-reservation repair during
resumed onboarding.
- [#6742](#6742) ->
`docs/about/release-notes.mdx`: Summarize pre-mutation resolution of
secret-free sandbox create intent.
- [#6721](#6721) ->
`docs/about/release-notes.mdx` and
`docs/get-started/quickstart-langchain-deepagents-code.mdx`: Record
bounded cleanup of completed managed Deep Agents headless sessions.
- [#6731](#6731) ->
`docs/about/release-notes.mdx` and
`docs/network-policy/customize-network-policy.mdx`: Document runtime
rejection of catch-all custom-policy destinations.
- [#6729](#6729) ->
`docs/about/release-notes.mdx` and `docs/get-started/prerequisites.mdx`:
Record the Node.js 22.19 minimum.
- [#6735](#6735) ->
`docs/about/release-notes.mdx` and
`docs/reference/platform-support.mdx`: Summarize the Ubuntu 26.04
userspace contract without claiming pending host or live validation.
- [#6775](#6775) ->
`docs/about/release-notes.mdx` and
`docs/resources/community-contributions.mdx`: Route independent
solutions outside canonical supported-product documentation.
- [#6740](#6740) ->
`docs/about/release-notes.mdx`: Summarize the semantic
dependency-upgrade contributor workflow.
- [#6777](#6777) ->
`docs/about/release-notes.mdx` and `docs/CONTRIBUTING.md`: Summarize the
route-safe documentation-refactor workflow.
- [#6741](#6741) ->
`docs/about/release-notes.mdx` and
`docs/security/openclaw-2026.6.10-dependency-review.md`: Summarize
reviewed npm archive verification and audit enforcement.
- [#6739](#6739) ->
`docs/about/release-notes.mdx` and
`docs/security/openclaw-2026.6.10-dependency-review.md`: Record the
locked offline dependency graph for the managed OpenClaw WeChat runtime.
- [#6737](#6737) ->
`docs/about/release-notes.mdx`: Record removal of the messaging build
plan from final OpenClaw and Hermes image environments.
- [#6733](#6733) ->
`docs/about/release-notes.mdx`: Summarize cached plugin dependency
layers for source and blueprint rebuilds.

### Skipped from docs-skip

- None. No commit or changed path in `v0.0.81..origin/main` matched
`openclaw-sandbox-permissive.yaml` or `config-show`, and the drafted
content contains none of the configured skip terms.

## 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 is a documentation-only
release-prep update; behavior is protected by the merged source PRs, and
the documentation build validates the changed routes and agent variants.
- [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 the DCO sign-off declaration 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 — tests are not applicable for this
documentation-only change.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: not run for this
documentation-only change.
- [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) — 0
errors; two pre-existing Fern warnings remain.
- [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)
— no new pages.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


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

## Summary by CodeRabbit

* **Documentation**
* Updated release notes with improvements to sandbox recovery,
onboarding, session management, policy validation, storage checks, and
system requirements.
  * Clarified Ollama setup instructions and status labels.
* Documented safer snapshot restoration, including dedicated ports and
protection against destructive failures.
* Expanded `backup-all` coverage to include eligible stopped sandboxes.
* Added guidance rejecting broad or catch-all network destinations in
custom policies.

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

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build area: local-models Local model providers, downloads, launch, or connectivity bug-fix PR fixes a bug or regression platform: container Affects Docker, containerd, Podman, or images platform: dgx-station Affects DGX Station hardware or workflows provider: vllm vLLM local or hosted provider behavior VDR Linked to VDR finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants