test(cli): replace slow subprocess matrices with unit coverage

[cv]

Summary

Replaces three subprocess-heavy CLI test matrices with focused unit coverage plus smaller smoke tests. This keeps the route-repair, host-alias probe, and sandbox mutation/help behaviors covered while reducing hook and CI runtime for the affected files.

Related Issue

Related to #4892

Changes

• Add injected route-repair and managed-route-reset helpers in src/lib/actions/sandbox/connect.ts with fast unit coverage.
• Add injected host-alias legacy-gateway probe/support seams and validator unit coverage.
• Move repeated sandbox command help/parser assertions into oclif adapter tests and trim subprocess smoke coverage.
• Focused timing results: route subprocess file ~74.3s in the prior CI artifact to 4.69s locally; host-alias subprocess file ~25.0s to 2.97s; sandbox-mutations subprocess file ~34.3s to 826ms.

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)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

• Tests

  • Broadened sandbox channel mutation tests to require a channel argument for all mutation commands.
  • Expanded CLI adapter tests to cover hosts and logs adapters, metadata, and flag validation.
  • Added extensive tests for route repair, managed inference reset, and legacy host-alias probing/validation.
  • Consolidated and refactored CLI fixtures and reduced redundant test cases.

• Refactor

  • Extracted repair/reset logic into dependency-injected functions for reuse and testing.
  • Refactored legacy host-alias probing and validation into injectable helpers and exported validators.

[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: aece79ad-6d5f-498c-abcf-f46e8de115fe

📥 Commits

Reviewing files that changed from the base of the PR and between 720eddc and 67a8381.

📒 Files selected for processing (4)

• src/commands/sandbox/channels/mutate.test.ts
• test/cli/sandbox-host-aliases.test.ts
• test/cli/sandbox-mutations.test.ts
• test/sandbox-connect-inference/route-swap-repair.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/commands/sandbox/channels/mutate.test.ts

---

📝 Walkthrough

Walkthrough

Refactors sandbox inference-route repair/reset and legacy host-alias probing to use dependency injection with new exported types and DI wrappers. Adds comprehensive unit tests for repair/reset and host-alias logic, shared CLI test fixtures, expanded CLI adapter tests (logs/hosts), and condensed/refactored CLI mutation tests.

Changes

Sandbox dependency-injection refactoring and testing

Layer / File(s)	Summary
Inference route repair/reset types and dependencies src/lib/actions/sandbox/connect.ts	Exports new types and dependency contracts for probing and repair/reset flows (SandboxInferenceRouteProbe, repair/reset deps).
Inference route repair/reset dependency-injected implementation src/lib/actions/sandbox/connect.ts	Adds repairSandboxInferenceRouteWithDeps and resetManagedInferenceRouteWithDeps with DI wiring; wrappers provide concrete implementations.
Inference route repair/reset unit test suite src/lib/actions/sandbox/connect-route-repair.test.ts	Vitest tests using makeRepairDeps/makeResetDeps to deterministically exercise probe/repair/reset scenarios and side effects.
Host-alias legacy gateway support dependency injection src/lib/actions/sandbox/host-aliases.ts	Refactors probe to DI form, exports LegacyGatewayHostAliasSupportDeps, assertLegacyGatewayHostAliasSupportWithDeps, probeLegacyGatewayContainerWithDeps, and exports validators for add/remove options.
Host-alias legacy gateway and validation unit tests src/lib/actions/sandbox/host-aliases.test.ts	Unit tests for probe classification (present/absent/unknown), driver-gated failures, and hostname/IP validation/normalization.
CLI command adapter coverage expansion src/commands/sandbox/oclif-command-adapters.test.ts	Adds hosts and logs adapters to tests, expands mocks (showSandboxLogs, host-alias actions), verifies logs options and oclif metadata, and asserts pre-dispatch flag rejection.
Channel mutation command argument validation src/commands/sandbox/channels/mutate.test.ts	Parametrized test ensuring add/remove/start/stop all require a channel argument and do not call action mocks when missing.
CLI test infrastructure consolidation and host-alias suite refactoring test/cli/sandbox-host-aliases.test.ts	Introduces makeCliFixture() and writeDockerStub() helpers and refactors host-alias CLI tests to use them; consolidates docker probe classification tests.
CLI mutations and test cleanup test/cli/sandbox-mutations.test.ts, test/sandbox-connect-inference/route-swap-repair.test.ts	Condenses CLI dispatch tests to focused public scenarios, updates imports, and adjusts route-swap-repair assertions and test case ordering.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4772: Overlapping changes around host-alias legacy-gateway probing and related types.
• NVIDIA/NemoClaw#4903: Related edits to sandbox mutation CLI tests and missing-channel behavior.
• NVIDIA/NemoClaw#4898: Adjacent CLI test refactors for sandbox host-aliases and mutations.

Suggested labels

NemoClaw CLI, area: cli, area: sandbox, area: e2e

Suggested reviewers

• prekshivyas
• cjagwani

Poem

🐇 I patch the sandbox, tidy flows with care,

Deps injected, probes queued in air,
Host aliases bowed, CLI stubs aligned,
Tests hop forward, reproducible and kind,
A rabbit’s cheer for code that's clean and fair!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.57% 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 PR title clearly summarizes the main change: replacing slow subprocess test matrices with faster unit coverage to improve CI/dev runtime.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/cli-unit-speedups

---

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

[github-actions]

PR Review Advisor

Findings: 1 needs attention, 4 worth checking, 0 nice ideas
Since last review: 4 prior items resolved, 3 still apply, 0 new items found

Review findings

🛠️ Needs attention

• Avoid adding more test seams to already-large sandbox action modules (src/lib/actions/sandbox/connect.ts:378): This PR adds dependency-injected helpers and exported types directly into existing high-risk sandbox action modules. Deterministic monolith analysis flags `connect.ts` growing from 1055 to 1149 lines and `host-aliases.ts` growing from 402 to 434 lines. These files sit on sandbox, Docker, Kubernetes, VM DNS, and local-inference boundaries, so additional in-place growth makes future security and correctness review harder.
  • Recommendation: Extract the route-repair/reset helpers and host-alias probe/validation seams into focused internal modules, or offset the growth by moving existing helper logic out of these monoliths before merging.
  • Evidence: `src/lib/actions/sandbox/connect.ts` gained exported `repairSandboxInferenceRouteWithDeps`, `resetManagedInferenceRouteWithDeps`, and dep/result types; `src/lib/actions/sandbox/host-aliases.ts` gained exported probe/support/validation seams.

🔎 Worth checking

• Source-of-truth review needed: Sandbox inference route repair and managed route reset: 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: `connect.ts` adds `repairSandboxInferenceRouteWithDeps` and `resetManagedInferenceRouteWithDeps`; tests cover behavior but not the source ownership/removal plan.
• Source-of-truth review needed: OpenShell VM DNS monkeypatch: 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: `applyOpenShellVmDnsMonkeypatch` performs guarded rootfs/init patching and `connect-route-repair.test.ts` covers monkeypatch success/fallback decisions.
• Document the source boundary and removal condition for inference route repair workarounds (src/lib/actions/sandbox/connect.ts:378): The route-repair/reset logic handles broken `inference.local` states through legacy DNS repair, VM DNS monkeypatching, non-legacy inference route reapply, and managed route reset. The invalid states and branch behavior are tested, but the code still does not clearly state why the OpenShell/gateway/local-inference source cannot be fixed here or when the recovery paths should be removed.
  • Recommendation: Add source-of-truth notes near the recovery helpers that identify the upstream/source boundary for each workaround and the removal condition, especially for managed route reset and VM DNS monkeypatch behavior.
  • Evidence: `connect-route-repair.test.ts` covers mocked recovery branches and `test/sandbox-connect-inference/route-swap-repair.test.ts` keeps targeted runtime smoke tests, but no clear removal condition is documented for the route repair/reset workaround as a whole.
• Keep a small runtime check for remaining unit-only route-repair fallback branches (test/sandbox-connect-inference/route-swap-repair.test.ts:1): The current diff restores the most important runtime checks for proxy bypasses, bearer-token non-leakage, VM DNS file writes, WSL Ollama pathing, Docker probe failures, and fail-closed connect behavior. A few security-sensitive fallback branches are now covered only through dependency-injected unit tests, which is weaker evidence for real OpenShell/Docker/VM/local-inference contracts.
  • Recommendation: Keep the fast unit tests, but add or identify one or two targeted runtime smoke cases for the remaining fallback branches listed in testDepth, rather than restoring the full slow matrices.
  • Evidence: `connect-route-repair.test.ts` unit-tests VM monkeypatch fallback and nonzero reset behavior with mocks, while the current runtime suite no longer exercises those exact real subprocess paths.

🌱 Nice ideas

• None.

Consider writing more tests for

• **Runtime validation** — Runtime validation — VM DNS monkeypatch applies, the post-patch `inference.local` probe remains broken, then `openshell inference set` is invoked and legacy DNS repair is not.. The PR restores the key high-risk runtime smoke coverage, but some fallback/recovery branches on sandbox, VM DNS, OpenShell inference, and local-provider boundaries are now covered only through mocked dependency-injected unit tests.
• **Runtime validation** — Runtime validation — nonzero `openshell inference set` during managed route reset still probes `inference.local` and proceeds only when that probe is healthy.. The PR restores the key high-risk runtime smoke coverage, but some fallback/recovery branches on sandbox, VM DNS, OpenShell inference, and local-provider boundaries are now covered only through mocked dependency-injected unit tests.
• **Runtime validation** — Runtime validation — local Ollama dependency failure stops before `openshell inference set` and before `openshell sandbox connect`.. The PR restores the key high-risk runtime smoke coverage, but some fallback/recovery branches on sandbox, VM DNS, OpenShell inference, and local-provider boundaries are now covered only through mocked dependency-injected unit tests.
• **Keep a small runtime check for remaining unit-only route-repair fallback branches** — Keep the fast unit tests, but add or identify one or two targeted runtime smoke cases for the remaining fallback branches listed in testDepth, rather than restoring the full slow matrices.
• **Acceptance clause:** Related to ci: safely split slow CLI coverage suites #4892 — add test evidence or identify existing coverage. Deterministic context reported `linkedIssues: []` and did not provide issue ci: safely split slow CLI coverage suites #4892 body/comments, so literal issue acceptance criteria could not be mapped.
• **Acceptance clause:** This keeps the route-repair, host-alias probe, and sandbox mutation/help behaviors covered while reducing hook and CI runtime for the affected files. — add test evidence or identify existing coverage. Host-alias probe and mutation/help coverage now have representative runtime smoke tests. Route-repair coverage keeps key runtime checks, but VM monkeypatch fallback-to-reapply and nonzero route-reset probe-success behavior are now unit-only.
• **Acceptance clause:** `npx prek run --all-files` passes — add test evidence or identify existing coverage. This is a PR-provided verification claim. The review used read-only inspection and did not execute commands.
• **Acceptance clause:** `npm test` passes — add test evidence or identify existing coverage. This is a PR-provided verification claim. The review used read-only inspection and did not execute commands.

Since last review details

Current findings:

• Source-of-truth review needed: Sandbox inference route repair and managed route reset: 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: `connect.ts` adds `repairSandboxInferenceRouteWithDeps` and `resetManagedInferenceRouteWithDeps`; tests cover behavior but not the source ownership/removal plan.
• Source-of-truth review needed: OpenShell VM DNS monkeypatch: 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: `applyOpenShellVmDnsMonkeypatch` performs guarded rootfs/init patching and `connect-route-repair.test.ts` covers monkeypatch success/fallback decisions.
• Avoid adding more test seams to already-large sandbox action modules (src/lib/actions/sandbox/connect.ts:378): This PR adds dependency-injected helpers and exported types directly into existing high-risk sandbox action modules. Deterministic monolith analysis flags `connect.ts` growing from 1055 to 1149 lines and `host-aliases.ts` growing from 402 to 434 lines. These files sit on sandbox, Docker, Kubernetes, VM DNS, and local-inference boundaries, so additional in-place growth makes future security and correctness review harder.
  • Recommendation: Extract the route-repair/reset helpers and host-alias probe/validation seams into focused internal modules, or offset the growth by moving existing helper logic out of these monoliths before merging.
  • Evidence: `src/lib/actions/sandbox/connect.ts` gained exported `repairSandboxInferenceRouteWithDeps`, `resetManagedInferenceRouteWithDeps`, and dep/result types; `src/lib/actions/sandbox/host-aliases.ts` gained exported probe/support/validation seams.
• Document the source boundary and removal condition for inference route repair workarounds (src/lib/actions/sandbox/connect.ts:378): The route-repair/reset logic handles broken `inference.local` states through legacy DNS repair, VM DNS monkeypatching, non-legacy inference route reapply, and managed route reset. The invalid states and branch behavior are tested, but the code still does not clearly state why the OpenShell/gateway/local-inference source cannot be fixed here or when the recovery paths should be removed.
  • Recommendation: Add source-of-truth notes near the recovery helpers that identify the upstream/source boundary for each workaround and the removal condition, especially for managed route reset and VM DNS monkeypatch behavior.
  • Evidence: `connect-route-repair.test.ts` covers mocked recovery branches and `test/sandbox-connect-inference/route-swap-repair.test.ts` keeps targeted runtime smoke tests, but no clear removal condition is documented for the route repair/reset workaround as a whole.
• Keep a small runtime check for remaining unit-only route-repair fallback branches (test/sandbox-connect-inference/route-swap-repair.test.ts:1): The current diff restores the most important runtime checks for proxy bypasses, bearer-token non-leakage, VM DNS file writes, WSL Ollama pathing, Docker probe failures, and fail-closed connect behavior. A few security-sensitive fallback branches are now covered only through dependency-injected unit tests, which is weaker evidence for real OpenShell/Docker/VM/local-inference contracts.
  • Recommendation: Keep the fast unit tests, but add or identify one or two targeted runtime smoke cases for the remaining fallback branches listed in testDepth, rather than restoring the full slow matrices.
  • Evidence: `connect-route-repair.test.ts` unit-tests VM monkeypatch fallback and nonzero reset behavior with mocks, while the current runtime suite no longer exercises those exact real subprocess paths.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

[github-actions]

E2E Advisor Recommendation

Required E2E: inference-routing-e2e, sandbox-operations-e2e
Optional E2E: cloud-inference-e2e, vm-driver-privileged-exec-routing-e2e

Dispatch hint: inference-routing-e2e,sandbox-operations-e2e

Auto-dispatched E2E: inference-routing-e2e, sandbox-operations-e2e via nightly-e2e.yaml at 67a8381cb27ed247a48a2b0f5c10b55bbdcf860d — nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• inference-routing-e2e (medium): Required because connect.ts changed the inference.local repair/reset flow. This E2E validates real sandbox-routed inference behavior and inference routing error/credential classifications against a live OpenShell sandbox.
• sandbox-operations-e2e (high): Required because sandbox connect behavior is on the critical sandbox lifecycle path. This job exercises live sandbox operations including connect/chat through inference.local, status/logging, and lifecycle resilience.

Optional E2E

• cloud-inference-e2e (medium): Useful additional confidence for the basic cloud onboard plus inference.local-from-sandbox path after the connect route-repair refactor, but less targeted than inference-routing-e2e.
• vm-driver-privileged-exec-routing-e2e (low): Optional because the changed connect repair code includes VM-driver DNS monkeypatch/route-reapply branches and host-alias support checks explicitly gate VM/direct-container drivers; this job gives adjacent VM-driver routing confidence.

New E2E recommendations

• sandbox-host-aliases (high): No existing E2E job appears to exercise real nemoclaw <sandbox> hosts-add/hosts-list/hosts-remove against a live legacy OpenShell gateway and verify Kubernetes Sandbox hostAliases persistence plus Docker probe failure modes.
  • Suggested test: Add a focused host-aliases E2E that onboards a legacy/kubernetes-driver sandbox, adds a host alias, verifies resolution or persisted Sandbox CR hostAliases, removes it, and covers the missing-gateway/Docker-probe user-facing errors without reaching kubectl exec.
• sandbox-connect-inference-route-repair (medium): Existing E2E jobs validate healthy inference.local routing, but the changed broken-route repair branches are mainly covered by unit/integration tests. A live failure-injection E2E would catch regressions in the actual openshell/docker/kubectl path.
  • Suggested test: Add a targeted connect route-repair E2E that creates a sandbox, deliberately breaks inference.local/DNS route state, runs nemoclaw <sandbox> connect --probe-only, and asserts the route is repaired or fails closed with actionable diagnostics.

Dispatch hint

• Workflow: nightly-e2e.yaml
• jobs input: inference-routing-e2e,sandbox-operations-e2e

[github-actions]

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw
Optional scenario E2E: ubuntu-repo-cloud-openclaw-repair, gpu-repo-local-ollama-openclaw

Dispatch required scenario E2E:

• gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required scenario E2E

• ubuntu-repo-cloud-openclaw: Product changes touch sandbox connect inference-route repair and sandbox host-alias command behavior. The smallest dispatchable scenario that exercises the repo install, sandbox startup, inference.local reachability from inside the sandbox, and baseline sandbox inspection/log flows is the canonical Ubuntu repo cloud OpenClaw scenario.
  • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional scenario E2E

• ubuntu-repo-cloud-openclaw-repair: Adjacent repair-oriented onboarding scenario for the same Ubuntu/OpenClaw surface. Useful if reviewers want extra coverage around repair-existing-config flows near the connect route-repair changes, but it is not the primary suite path for these files.
  • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw-repair
• gpu-repo-local-ollama-openclaw: Optional special-runner coverage for local Ollama inference-route behavior, since connect route repair includes local-provider dependency handling. Keep optional because it requires the GPU runner and the primary changed surface is covered by the Ubuntu cloud scenario.
  • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=gpu-repo-local-ollama-openclaw

Relevant changed files

• src/lib/actions/sandbox/connect.ts
• src/lib/actions/sandbox/host-aliases.ts

[cv]

use it.each table tests instead of an internal for loop here

[cv]

Done in 67a8381: converted the internal loop to it.each table tests.

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27095299976
Target ref: 720eddcc9d939720fc6193fd37a1c56c53db49d9
Workflow ref: main
Requested jobs: inference-routing-e2e,sandbox-operations-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job	Result
inference-routing-e2e	✅ success
sandbox-operations-e2e	⚠️ cancelled

[cv]

Addressed the review/advisor feedback in commit 67a8381.

What changed:

• Converted the channel missing-arg test loop to an it.each table.
• Restored targeted host-alias runtime smoke coverage for real Docker ENOENT, docker ps timeout, and nonzero daemon failures, each proving we do not fall through to docker exec ... kubectl.
• Restored public compatibility route smoke coverage for representative sandbox help families, policy/channel dry-runs, and a native sandbox ... command form.
• Restored targeted route-repair runtime smoke coverage for VM DNS monkeypatch filesystem effects, fail-closed before openshell sandbox connect, local Ollama proxy/token handling, and WSL host-path proxy handling.

Verification:

• npm run build:cli
• Focused Vitest set covering the changed files: 9 files, 76 tests passed in 28.10s
• npm run test-size:check
• npm run source-shape:check
• Commit and pre-push hooks passed, including CLI tests and typecheck.

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27095641603
Target ref: b552a7be76fc0b463e20ab47a02a7d919abee06b
Workflow ref: main
Requested jobs: sandbox-operations-e2e,inference-routing-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job	Result
inference-routing-e2e	✅ success
sandbox-operations-e2e	✅ success

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27095907538
Target ref: 67a8381cb27ed247a48a2b0f5c10b55bbdcf860d
Workflow ref: main
Requested jobs: inference-routing-e2e,sandbox-operations-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job	Result
inference-routing-e2e	✅ success
sandbox-operations-e2e	✅ success