test(e2e): accept managed vLLM recovery inspection

[cjagwani]

Summary

The vLLM Docker storage E2E proxy now accepts the read-only managed-container inspection that recovery performs before deciding whether to reuse or recreate the local vLLM container. The proxy remains fail-closed for any other container name.

Main E2E evidence

• Affected run: 31140352904
• Affected job: 92751286275 (Shared E2E (vllm-docker-storage))
• Failure: the E2E command proxy rejected docker container inspect nemoclaw-vllm, even though the recovery implementation now performs that read-only inspection.
• Ownership search: no open PR claimed this exact vLLM Docker storage command-contract failure when the fix was opened.

Changes

• Accept docker container inspect only for the exact managed vLLM container name.
• Keep similarly prefixed or otherwise different container names rejected.
• Assert the positive command and a bounded negative proxy probe so the regression cannot hang the test process.

Type of Change

• Code change (test-contract fix)
• Code change with doc updates
• Doc only

Quality Gates

• Tests added or updated for changed behavior
• Docs not applicable — test-only Docker proxy expectations; no user-facing behavior or supported surface changes
• Sensitive paths changed
• Non-success, skipped, or missing CI check accepted by maintainer

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: exact committed diff is limited to test/vllm-docker-storage.test.ts (+20/-3) and changes no production or user-facing behavior.
• Agent: Codex Desktop

Verification

• PR description includes a Signed-off-by: line and every PR commit is signed/verified
• Normal commit and push hooks passed
• npx biome check test/vllm-docker-storage.test.ts
• npm run build:cli
• npm --prefix nemoclaw run build
• npm run typecheck
• npm run test:projects:check
• npm run test:e2e-phases:check (122 tests across 79 files)
• Targeted integration file collects and skips without the opt-in flag; the real-Docker path is intentionally guarded against unsupported macOS execution and runs in Linux E2E CI
• No coverage or assertion was weakened

---

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

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

The vLLM Docker storage test now permits container inspect only for the managed vLLM container. It verifies allowed inspection and blocks inspection of other containers.

Changes

vLLM Docker validation

Layer / File(s)	Summary
Restrict and verify container inspection test/vllm-docker-storage.test.ts	The test imports the managed container name. The Docker proxy allowlist accepts inspection only for that container. Assertions cover the allowed command and reject other container names with status 97.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 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 clearly and concisely identifies the main change: accepting the managed vLLM recovery inspection in the E2E test.

✨ 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 codex/fix-vllm-docker-storage-contract

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 3b0ebe0 in the codex/fix-vllm-docke... branch remains at 96%, unchanged from commit cc4b816 in the main branch.

---

_{Updated August 07, 2026 08:06 UTC}

[coderabbitai]

🧹 Nitpick comments (1)

test/vllm-docker-storage.test.ts (1)

253-259: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a negative assertion for non-managed container inspection.

This assertion proves that nemoclaw-vllm is inspected. It does not prove that another inspection target is rejected. If the proxy later permits every container inspect call, this test still passes. Add a proxy-level negative probe and assert the existing rejection status for a different container name.

As per path instructions, review tests for behavioral confidence rather than implementation lock-in.

🤖 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 `@test/vllm-docker-storage.test.ts` around lines 253 - 259, Extend the test
around the existing docker command assertions to issue a proxy-level container
inspect request for a different container name, then assert the established
rejection status. Keep the positive HOST_LOCAL_VLLM_CONTAINER_NAME assertion and
validate behavior through the proxy rather than adding implementation-specific
checks.

Source: Path instructions

🤖 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 `@test/vllm-docker-storage.test.ts`:
- Around line 253-259: Extend the test around the existing docker command
assertions to issue a proxy-level container inspect request for a different
container name, then assert the established rejection status. Keep the positive
HOST_LOCAL_VLLM_CONTAINER_NAME assertion and validate behavior through the proxy
rather than adding implementation-specific checks.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5f7ff173-1b62-4e6e-bceb-71f5654d87da

📥 Commits

Reviewing files that changed from the base of the PR and between c11f455 and 6449003.

📒 Files selected for processing (1)

• test/vllm-docker-storage.test.ts

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized terminology decisions match; normalized E2E selections match; severity counts match.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: vllm-docker-storage

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[cjagwani]

Addressed the CodeRabbit negative-coverage finding in verified commit 9faf7a6: the E2E now invokes the Docker proxy with a different container name and asserts the established rejection status, with the required timeout and SIGKILL bounds. Formatting, type checking, semantic E2E phase validation, normal hooks, and test collection pass.

[prekshivyas]

Current-main refresh is now visible and the prior approval remains. The refreshed DCO check is failing because the PR description is empty; the gate expects a Signed-off-by: Name <email> line in the description. I have not modified the branch or PR body.