test(e2e): add fixture-friendly clients

[cv]

Summary

Adds fixture-friendly client helpers that let Vitest scenarios talk through named NemoClaw concepts instead of raw shell snippets. Draft stack PR 4/7. cc @jyaunches for review.

Related Issue

Refs #4941.

Changes

• Adds command, host, gateway, sandbox, provider, and state clients under test/e2e-scenario/framework/clients/.
• Wires those clients into the scenario fixture context.
• Adds framework tests covering command dispatch, cleanup registration, and client behavior.

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
  • Added comprehensive end-to-end testing infrastructure with new client utilities for testing framework operations across command execution, gateway health checks, sandbox management, provider integration, and state file handling.

[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]

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: 25b69207-78e2-4755-9af0-eee3f9d5d8cd

📥 Commits

Reviewing files that changed from the base of the PR and between 0151ff3 and f9991c2.

📒 Files selected for processing (9)

• test/e2e-scenario/framework-tests/e2e-clients.test.ts
• test/e2e-scenario/framework/clients/command.ts
• test/e2e-scenario/framework/clients/gateway.ts
• test/e2e-scenario/framework/clients/host.ts
• test/e2e-scenario/framework/clients/index.ts
• test/e2e-scenario/framework/clients/provider.ts
• test/e2e-scenario/framework/clients/sandbox.ts
• test/e2e-scenario/framework/clients/state.ts
• test/e2e-scenario/framework/e2e-test.ts

---

📝 Walkthrough

Walkthrough

This PR introduces a complete E2E test framework client library with shell command execution abstractions, specialized clients for CLI/gateway/sandbox/provider operations, endpoint validation, file state reading, and a comprehensive test suite validating all client behaviors.

Changes

E2E Test Framework Clients

Layer / File(s)	Summary
Command Runner interface & helpers test/e2e-scenario/framework/clients/command.ts	CommandRunner interface for executing trusted shell commands; assertExitZero helper throws on non-zero exit with stderr/stdout/signal fallback; artifactLabel normalizes strings into lowercased artifact names with "request" default.
Host CLI Client test/e2e-scenario/framework/clients/host.ts	HostCliClient wraps CommandRunner to execute nemoclaw CLI with binary path resolution (cliPath option, NEMOCLAW_CLI_BIN, or "nemoclaw" default), optional cwd application, and convenience methods: command() for arbitrary CLI invocation, nemoclaw() with artifact naming, expectNemoclawAvailable() for availability checks.
Gateway & Sandbox Clients test/e2e-scenario/framework/clients/gateway.ts, test/e2e-scenario/framework/clients/sandbox.ts	GatewayClient wraps HostCliClient for status() and expectHealthy() checks; SandboxClient wraps CommandRunner for sandbox operations with regex-validated sandbox names, providing list()/status()/exec()/expectRunning() methods and artifact naming.
Provider Client with trusted endpoint validation test/e2e-scenario/framework/clients/provider.ts	TrustedProviderEndpoint enforces URL safety: only http/https schemes, no credentials/blocked hosts, rejects private/link-local IPs except loopback, requires allowedHosts for external hosts; computes deterministic artifact/log labels and query-based redaction values. ProviderClient executes curl against validated endpoints, merges redaction values, asserts success, and parses JSON responses.
State Client test/e2e-scenario/framework/clients/state.ts	StateClient reads and interprets state files: exists() checks presence (false only for missing path), readText() reads UTF-8 content, readJson() parses JSON with error handling.
Barrel exports & fixture integration test/e2e-scenario/framework/clients/index.ts, test/e2e-scenario/framework/e2e-test.ts	Index re-exports all public client types and implementations; extends E2EScenarioFixtures with host/gateway/sandbox/provider/state fixtures and wires Vitest fixture setup to instantiate clients from appropriate upstreams.
Test suite with FakeRunner test/e2e-scenario/framework-tests/e2e-clients.test.ts	Comprehensive Vitest suite with FakeRunner test double recording shell commands; validates CLI argument/option construction (cwd/env/timeout), sandbox name validation, provider endpoint JSON parsing without redirects, URL safety rules (schemes/hosts/userinfo/private/link-local), credential redaction in artifact names/redaction lists, error message safety (no response bodies/query strings), loopback IP allowance, and StateClient file operations including null byte rejection.

Sequence Diagram

sequenceDiagram
  participant Test as E2E Test
  participant Host as HostCliClient
  participant Runner as CommandRunner
  participant Shell as Shell/nemoclaw
  
  Test->>Host: nemoclaw(['gateway', 'status'])
  Host->>Host: resolve binary path
  Host->>Runner: run(trustedShellCommand)
  Runner->>Shell: execute nemoclaw gateway status
  Shell-->>Runner: ShellProbeResult
  Runner-->>Host: ShellProbeResult
  Host-->>Test: ShellProbeResult (artifact: gateway-status)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4965: Extends the E2E test framework fixture context by wiring the new client fixtures into the test infrastructure.

Suggested reviewers

• jyaunches
• prekshivyas

Poem

🐰 A CLI framework springs to life with tested grace,
Command runners dancing, fixtures finding their place,
Sandboxes validated, endpoints held secure,
State files read with confidence pure. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 'test(e2e): add fixture-friendly clients' accurately summarizes the main change: introducing new client helpers for e2e testing that integrate with Vitest fixtures.
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/e2e-4941-stack-04-clients

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No existing E2E job is required. This PR only adds/updates E2E scenario test framework fixtures and their Vitest coverage; it does not modify runtime/user-flow code or E2E scenario definitions/manifests that would change installer, onboarding, sandbox lifecycle, credentials, network policy, inference routing, deployment, or real assistant behavior.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Scenario Advisor Recommendation

Required scenario E2E: e2e-scenarios-all
Optional scenario E2E: None

Dispatch required scenario E2E:

• gh workflow run e2e-scenarios-all.yaml --ref <pr-head-ref>

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

• e2e-scenarios-all: Shared scenario E2E framework fixtures and clients were added/changed under test/e2e-scenario/framework. These are common runtime/test harness code used across scenario suites, so the full scenario fan-out should run.
  • Dispatch: gh workflow run e2e-scenarios-all.yaml --ref <pr-head-ref>

Optional scenario E2E

• None.

Relevant changed files

• test/e2e-scenario/framework-tests/e2e-clients.test.ts
• test/e2e-scenario/framework/clients/command.ts
• test/e2e-scenario/framework/clients/gateway.ts
• test/e2e-scenario/framework/clients/host.ts
• test/e2e-scenario/framework/clients/index.ts
• test/e2e-scenario/framework/clients/provider.ts
• test/e2e-scenario/framework/clients/sandbox.ts
• test/e2e-scenario/framework/clients/state.ts
• test/e2e-scenario/framework/e2e-test.ts

[github-actions]

PR Review Advisor

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

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: Provider endpoint hostname policy: 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: allowedHosts is compared only against url.hostname in provider.ts:131-136, while curl later receives endpoint.url at provider.ts:159.
• Provider hostname allowlist does not validate resolved IPs (test/e2e-scenario/framework/clients/provider.ts:131): trustedProviderEndpoint blocks unsafe IP literals and requires allowedHosts for external HTTPS endpoints, but curl performs DNS resolution later. If an allowed hostname is attacker-controlled, compromised, or DNS-rebindable, the request can still connect to loopback, private, link-local, or metadata IPs that were never checked by the endpoint policy.
  • Recommendation: Either resolve allowed hostnames before the request and reject non-global or metadata destinations, use a curl/connect path that pins validated resolved addresses, or document/enforce that allowedHosts may only contain immutable framework-owned provider domains. Add a regression test for an allowed HTTPS hostname resolving to a blocked internal address.
  • Evidence: The policy maps and compares allowedHosts at provider.ts:131-136 and only calls isPrivateOrLinkLocalIp(host) on the URL hostname at provider.ts:125. ProviderClient then runs curl with the original endpoint URL at provider.ts:159, leaving final DNS resolution to curl.

🌱 Nice ideas

• None.

Consider writing more tests for

• **Runtime validation** — Provider endpoint rejects an allowed HTTPS hostname that resolves to loopback, private, link-local, or metadata IPs before curl connects.. Unit coverage is strong for command construction, provider URL syntax checks, redaction, and client behavior. Because ProviderClient creates a real network boundary through curl, runtime or boundary-level validation would improve confidence in the SSRF policy for DNS-backed allowed hosts.
• **Runtime validation** — Provider endpoint rejects representative special non-global IP literals, including IPv4-mapped private IPv6 forms and reserved or multicast ranges if the intended policy is global-only external HTTPS.. Unit coverage is strong for command construction, provider URL syntax checks, redaction, and client behavior. Because ProviderClient creates a real network boundary through curl, runtime or boundary-level validation would improve confidence in the SSRF policy for DNS-backed allowed hosts.
• **Runtime validation** — Fixture context exposes host, gateway, sandbox, provider, and state clients to an e2eTest callback.. Unit coverage is strong for command construction, provider URL syntax checks, redaction, and client behavior. Because ProviderClient creates a real network boundary through curl, runtime or boundary-level validation would improve confidence in the SSRF policy for DNS-backed allowed hosts.
• **Acceptance clause:** Refs Adopt Vitest fixtures as the E2E scenario execution model #4941. — add test evidence or identify existing coverage. The deterministic context did not include linked issue Adopt Vitest fixtures as the E2E scenario execution model #4941 body or comments, so no literal issue acceptance clauses could be mapped. Repo migration docs identify Adopt Vitest fixtures as the E2E scenario execution model #4941 as the Vitest fixture execution-model decision, and this diff moves in that direction by adding fixture clients.
• **Provider endpoint hostname policy** — Add a provider fixture test where an allowed HTTPS hostname resolves to a blocked internal address and verify the request is rejected before curl can connect, or add a contract test proving such hostnames cannot enter allowedHosts.. allowedHosts is compared only against url.hostname in provider.ts:131-136, while curl later receives endpoint.url at provider.ts:159.

Since last review details

Current findings:

• Source-of-truth review needed: Provider endpoint hostname policy: 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: allowedHosts is compared only against url.hostname in provider.ts:131-136, while curl later receives endpoint.url at provider.ts:159.
• Provider hostname allowlist does not validate resolved IPs (test/e2e-scenario/framework/clients/provider.ts:131): trustedProviderEndpoint blocks unsafe IP literals and requires allowedHosts for external HTTPS endpoints, but curl performs DNS resolution later. If an allowed hostname is attacker-controlled, compromised, or DNS-rebindable, the request can still connect to loopback, private, link-local, or metadata IPs that were never checked by the endpoint policy.
  • Recommendation: Either resolve allowed hostnames before the request and reject non-global or metadata destinations, use a curl/connect path that pins validated resolved addresses, or document/enforce that allowedHosts may only contain immutable framework-owned provider domains. Add a regression test for an allowed HTTPS hostname resolving to a blocked internal address.
  • Evidence: The policy maps and compares allowedHosts at provider.ts:131-136 and only calls isPrivateOrLinkLocalIp(host) on the URL hostname at provider.ts:125. ProviderClient then runs curl with the original endpoint URL at provider.ts:159, leaving final DNS resolution to curl.

Workflow run details

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