Run managed vLLM Docker with argv

[fallintoplace]

Summary

The managed vLLM startup path built docker run as one shell string by joining profile flags, Hugging Face token flags, image/container fields, and the cache mount before executing through bash -c. That made profile-controlled values and host paths more fragile than they needed to be, and created an avoidable host-side command injection surface.

This changes the long-lived vLLM container startup to build Docker argv tokens and execute through the existing Docker adapter helpers. It also moves container cleanup from shell strings to Docker argv helpers, resolves the Hugging Face cache mount with os.homedir(), and removes the Station multi-GPU shell quote workaround now that --gpus device=0,1 is passed as a single argv token.

The in-container /bin/bash -lc boundary remains intentional for the existing pip install ... && vllm serve ... behavior; the host Docker invocation no longer depends on shell parsing.

Validation

• npm test -- src/lib/inference/vllm.test.ts src/lib/inference/vllm-models.test.ts
• npm test -- test/detect-vllm-profile.test.ts
• npm test -- src/lib/inference/vllm.test.ts
• npx @biomejs/biome lint src/lib/inference/vllm.ts src/lib/inference/vllm.test.ts
• npm run build:cli
• npm run typecheck:cli

Summary by CodeRabbit

• Refactor
  • Improved vLLM container initialization to use safer Docker argument handling, replacing shell-based command construction with direct argument arrays for enhanced reliability.
  • Enhanced Hugging Face token forwarding and cache directory mounting to work more consistently across platform configurations.
  • Strengthened GPU selection integration with vLLM container startup.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 687b247c-a8cd-47ca-a7f2-36d3d1893155

📥 Commits

Reviewing files that changed from the base of the PR and between 543bbd4 and 0c85ca1.

📒 Files selected for processing (2)

• src/lib/inference/vllm.test.ts
• src/lib/inference/vllm.ts

---

📝 Walkthrough

Walkthrough

The PR refactors vLLM container launch in vllm.ts from shell-string command construction to an argv-based approach. It introduces shared Hugging Face cache mount helpers, replaces the exported buildVllmRunCommand with buildVllmRunArgs (with argument validation), migrates container start/stop operations to typed Docker adapter functions, and updates tests accordingly.

Changes

vLLM Docker argv refactor

Layer / File(s)	Summary
HF cache helpers and shared run flags src/lib/inference/vllm.ts	Adds HF_CACHE_CONTAINER_DIR, hfCacheMount(), and vllmDockerRunFlags() to centralize GPU/IPC/cache-mount/HF_HOME flag generation; updates imports to use os/path and new Docker adapters; switches hf download to the shared cache mount helpers; refines the HF token forwarding comment.
buildVllmRunArgs: validated argv construction src/lib/inference/vllm.ts, src/lib/inference/vllm.test.ts	Removes buildVllmRunCommand and adds buildVllmRunArgs with validateDockerArg(s) guards, constructing a full docker run argv array with --restart unless-stopped, port mapping, container name, and bash -lc serve command. Tests assert restart policy, metadata, custom flags, shell-metacharacter safety, HF cache mount formatting, and Station multi-GPU rendering.
Container startup and teardown via typed Docker adapters src/lib/inference/vllm.ts	Startup calls dockerForceRm before launch; buildVllmRunArgs output is passed to dockerRunDetached with buildHfTokenForwardEnv() in runner options; installVllm failure-path teardown switches from runShell('docker stop ...') to dockerStop(profile.containerName).
Test infrastructure and installVllm integration test src/lib/inference/vllm.test.ts	Expands hoisted mocks to include dockerForceRm, dockerRunDetached, dockerSpawn, and dockerStop; adds mockDockerSpawnSuccess() helper; drops runShell mock; adds an installVllm test verifying cleanup, argv correctness, no shell string leakage, vllm serve in the -lc payload, and { env: { HF_TOKEN } } options wiring.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• NVIDIA/NemoClaw#5038: Adds resolveVllmInstallModel and platform-scoped model defaults to vllm.ts, which the new buildVllmRunArgs and installVllm changes in this PR build upon directly.

Suggested labels

provider: vllm, area: onboarding, v0.0.63

🐇 From shell strings I now leap free,
Each argv token safe as can be!
--restart unless-stopped, port and name—
No metachar mischief, no quoting game.
HF cache mounts tidy, Docker adapters true,
This bunny hops safely through each docker run queue! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Run managed vLLM Docker with argv' directly and accurately summarizes the main change: refactoring vLLM Docker invocation from shell string construction to argv-based execution.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[wscurran]

✨ Thanks for the proposed fix addressing the shell-string-based Docker invocation that created a command injection surface in the managed vLLM startup path. This proposes a way to build Docker argv tokens and execute through Docker adapter helpers, moving container cleanup to argv helpers and resolving the Hugging Face cache mount with os.homedir().

[apurvvkumaria]

Superseded by #6523, which rebases this two-file security hardening onto current main with a clean verified/DCO-compliant history. Minh Vu's implementation credit is preserved in the PR body and as Co-authored-by on the signed commit.