Walkthrough

Replaces log-stream readiness detection with polling of the vLLM OpenAI-compatible /v1/models endpoint, simplifies VllmProfile to timeout-based fields, adds heartbeat messages during download and launch, requires curl for readiness checks, and prints container log tails on launch failures.

Changes

vLLM Readiness Mechanism Replacement

Sequence Diagram(s)

sequenceDiagram
  participant Install as installVllm
  participant Poll as waitForVllmReady
  participant Endpoint as /v1/models Endpoint
  participant Container as vLLM Container
  participant Logs as Container Logs
  
  Install->>Poll: start polling with timeout
  Poll->>Container: docker inspect (running?)
  Container-->>Poll: running or not
  Poll->>Endpoint: curl /v1/models
  Endpoint-->>Poll: success or network error
  alt ready
    Poll-->>Install: {ok: true}
  else timeout reached
    Poll->>Logs: tail container logs
    Logs-->>Poll: recent log lines
    Poll-->>Install: {ok: false} + log tail
  else container stopped
    Poll-->>Install: {ok: false}
  else heartbeat interval
    Poll-->>Poll: emit "API not ready" message
    Poll->>Endpoint: retry after interval
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4619: Modifies src/lib/inference/vllm.ts vLLM installation/launch flow and image/model selection; relates to endpoint-based readiness and docker invocation changes.

Suggested labels

Provider: vLLM

Poem

🐰 I watched the logs and hopped about the glen,
Now I poll the endpoint, again and again—
A heartbeat ping, a gentle cheer,
Tail the logs when doom is near,
Hop, install, vLLM's live—let's run!

❌ Failed checks (1 warning)

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

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

• gpu-e2e (high): Closest existing GPU/local-inference E2E. It exercises a real NVIDIA GPU host, Docker, install/onboard, local inference startup, sandbox creation, and live inference through the sandbox. Although it uses Ollama rather than vLLM, this PR changes the managed GPU local-inference install/readiness path and should receive GPU-host coverage.
• inference-routing-e2e (medium): Validates inference.local routing, provider route health, credential isolation, and error classification against a live sandbox. Managed vLLM install ultimately registers vllm-local as an OpenAI-compatible provider and relies on the same gateway/sandbox inference route behavior.

Optional E2E

• onboard-inference-smoke-e2e (low): Hermetic regression guard that onboarding must not report success until a configured inference route serves a real request. Adjacent to this PR’s switch from log-marker readiness to endpoint readiness, but it does not run managed vLLM itself.
• cloud-onboard-e2e (medium): Useful broad confidence that the onboarding flow still installs and configures a sandbox from source after changes in an onboarding-imported inference module, though it does not exercise local vLLM.

New E2E recommendations

• managed vLLM install on GPU/Spark (high): No existing E2E appears to run NEMOCLAW_PROVIDER=install-vllm or interactively select install-vllm on a GPU/Spark/Station host. The changed code path covers Docker pull, hf download, container startup, /v1/models readiness polling, model detection, and vllm-local route setup, but current GPU E2E is Ollama-only.
  • Suggested test: Add a managed-vllm-install-e2e job on a GPU runner using a small supported vLLM model/cache, then assert /v1/models readiness, vllm-local provider registration, and a sandbox inference.local chat completion.
• vLLM readiness failure diagnostics (medium): The diff replaces docker log streaming/fatal-marker detection with endpoint polling plus tail-on-failure. There is no apparent E2E/regression job that simulates curl missing, container exit before readiness, timeout, or malformed /v1/models response and asserts actionable diagnostics.
  • Suggested test: Add a hermetic regression E2E for vLLM readiness failure modes with mocked docker/curl commands, verifying non-zero exit, container cleanup, and printed log tail without leaking HF tokens.

Dispatch hint

• Workflow: .github/workflows/nightly-e2e.yaml
• jobs input: gpu-e2e,inference-routing-e2e

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

• None. The PR changes the vLLM install/profile helper and its unit test, but the current scenario E2E catalog and ROUTES table do not include a vLLM onboarding scenario. Existing dispatchable scenarios cover cloud NVIDIA/OpenAI-compatible and local Ollama paths, so no scenario E2E job directly exercises this changed surface.

Optional scenario E2E

• None.

Relevant changed files

• src/lib/inference/vllm.ts

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: src/lib/inference/vllm.ts vllmEndpointReady() tolerant JSON parse: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: vllmEndpointReady() catches JSON.parse failures and returns false; test/detect-vllm-profile.test.ts does not exercise this path.
• Redact HF and vLLM logs before printing (src/lib/inference/vllm.ts:286): The changed progress and failure paths write Hugging Face download stdout/stderr and Docker log tails directly to the terminal. Those streams originate from third-party/container processes that receive forwarded Hugging Face credentials, so bearer tokens, signed URLs, proxy credentials, or environment dumps could bypass the repository's existing runner redaction.
  • Recommendation: Pass streamed chunks and stored tail/log-tail lines through the existing redaction utility, or route these outputs through a shared redacting logger. Add a regression test proving token-shaped strings in HF/vLLM output are not emitted verbatim.
  • Evidence: downloadModel() calls stream.write(buf) for stdout/stderr chunks and later prints raw hf tail lines; printContainerLogTail() writes each docker logs line directly with process.stderr.write(...). Existing redaction helpers are available in src/lib/security/redact.ts but are not used here.
• Add coverage for the new readiness and failure paths (test/detect-vllm-profile.test.ts:69): The PR replaces log-marker readiness detection with /v1/models polling and adds tolerant JSON parsing, container-exit handling, load timeouts, download heartbeats, and failure log tails, but the test change only updates profile-shape assertions after removing marker fields. The main new behavior and negative paths are unverified.
  • Recommendation: Add focused tests or injectable seams for vllmEndpointReady()/waitForVllmReady() covering curl failure, empty output, malformed JSON, JSON without a data array, successful data-array responses, container exit before readiness, timeout, and failure log-tail behavior. Include runtime/integration validation for the Docker + vLLM installer path.
  • Evidence: test/detect-vllm-profile.test.ts changes the removed ready/fatal marker assertion into a timeout-budget assertion, but no tests exercise the new curl readiness probe, wait loop, timeout handling, container-exit handling, or failure log printing in src/lib/inference/vllm.ts.
• Source-of-truth review needed for tolerant readiness JSON parsing (src/lib/inference/vllm.ts:376): vllmEndpointReady() treats malformed non-empty /v1/models output as simply not ready by catching JSON.parse failures and returning false. That localized tolerance may be appropriate for transient startup responses, but the code does not establish what invalid state creates the malformed response, why the source cannot be fixed, what regression test protects the behavior, or when the workaround can be removed.
  • Recommendation: Document or encode the expected invalid state at the source boundary, and add a regression test showing malformed readiness output is handled intentionally and can recover to a valid response. If the malformed response comes from a controllable source in this PR, prefer making that invalid state impossible instead of silently tolerating it.
  • Evidence: vllmEndpointReady() parses curl output from http://127.0.0.1:<VLLM\_PORT>/v1/models and catches JSON.parse errors with catch { return false; }, while the changed test file does not cover this path.

🌱 Nice ideas

• None.

Current findings:

• Source-of-truth review needed: src/lib/inference/vllm.ts vllmEndpointReady() tolerant JSON parse: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: vllmEndpointReady() catches JSON.parse failures and returns false; test/detect-vllm-profile.test.ts does not exercise this path.
• Redact HF and vLLM logs before printing (src/lib/inference/vllm.ts:286): The changed progress and failure paths write Hugging Face download stdout/stderr and Docker log tails directly to the terminal. Those streams originate from third-party/container processes that receive forwarded Hugging Face credentials, so bearer tokens, signed URLs, proxy credentials, or environment dumps could bypass the repository's existing runner redaction.
  • Recommendation: Pass streamed chunks and stored tail/log-tail lines through the existing redaction utility, or route these outputs through a shared redacting logger. Add a regression test proving token-shaped strings in HF/vLLM output are not emitted verbatim.
  • Evidence: downloadModel() calls stream.write(buf) for stdout/stderr chunks and later prints raw hf tail lines; printContainerLogTail() writes each docker logs line directly with process.stderr.write(...). Existing redaction helpers are available in src/lib/security/redact.ts but are not used here.
• Add coverage for the new readiness and failure paths (test/detect-vllm-profile.test.ts:69): The PR replaces log-marker readiness detection with /v1/models polling and adds tolerant JSON parsing, container-exit handling, load timeouts, download heartbeats, and failure log tails, but the test change only updates profile-shape assertions after removing marker fields. The main new behavior and negative paths are unverified.
  • Recommendation: Add focused tests or injectable seams for vllmEndpointReady()/waitForVllmReady() covering curl failure, empty output, malformed JSON, JSON without a data array, successful data-array responses, container exit before readiness, timeout, and failure log-tail behavior. Include runtime/integration validation for the Docker + vLLM installer path.
  • Evidence: test/detect-vllm-profile.test.ts changes the removed ready/fatal marker assertion into a timeout-budget assertion, but no tests exercise the new curl readiness probe, wait loop, timeout handling, container-exit handling, or failure log printing in src/lib/inference/vllm.ts.
• Source-of-truth review needed for tolerant readiness JSON parsing (src/lib/inference/vllm.ts:376): vllmEndpointReady() treats malformed non-empty /v1/models output as simply not ready by catching JSON.parse failures and returning false. That localized tolerance may be appropriate for transient startup responses, but the code does not establish what invalid state creates the malformed response, why the source cannot be fixed, what regression test protects the behavior, or when the workaround can be removed.
  • Recommendation: Document or encode the expected invalid state at the source boundary, and add a regression test showing malformed readiness output is handled intentionally and can recover to a valid response. If the malformed response comes from a controllable source in this PR, prefer making that invalid state impossible instead of silently tolerating it.
  • Evidence: vllmEndpointReady() parses curl output from http://127.0.0.1:<VLLM\_PORT>/v1/models and catches JSON.parse errors with catch { return false; }, while the changed test file does not cover this path.