Skip to content

test(e2e): migrate agent turn latency to vitest - #5554

Merged
cv merged 19 commits into
mainfrom
e2e-migrate/test-agent-turn-latency
Jun 21, 2026
Merged

test(e2e): migrate agent turn latency to vitest#5554
cv merged 19 commits into
mainfrom
e2e-migrate/test-agent-turn-latency

Conversation

@cv

@cv cv commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates the real agent turn latency E2E into a typed live Vitest scenario. The new test onboards OpenClaw and Hermes against hosted inference, verifies their managed inference.local configuration, and times one real model-backed turn through each runtime.

Related Issue

Refs #5098

Changes

  • Add a typed live Vitest replacement for test/e2e/test-agent-turn-latency-e2e.sh.
  • Wire a free-standing dispatchable Vitest job into .github/workflows/e2e-vitest-scenarios.yaml.
  • Preserve legacy shell deletion and any legacy shell workflow cleanup for Phase 11 per Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 migration governance.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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)

Targeted commands run:

  • npx biome check --write test/e2e-scenario/live/agent-turn-latency.test.ts
  • NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/agent-turn-latency.test.ts -t __compile_only_nomatch__ --silent=false --reporter=default --passWithNoTests
  • npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts
  • npx tsx scripts/check-test-file-size-budget.ts test/e2e-scenario/live/agent-turn-latency.test.ts
  • npx tsc --noEmit --strict --moduleResolution bundler --module preserve --target ES2022 --types node --allowImportingTsExtensions test/e2e-scenario/live/agent-turn-latency.test.ts
  • git diff --check

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

Summary by CodeRabbit

  • Tests

    • Replaced the prior shell-based latency check with a gated Live E2E Vitest test that runs two sequential hosted-inference turns (OpenClaw, then Hermes).
    • Validates routing/config, checks sandbox readiness, measures wall-clock latency against the configured limit, asserts expected model output, writes run/results artifacts, and performs best-effort sandbox cleanup.
  • CI/CD

    • Added a dedicated free-standing Vitest CI job for the latency scenario (conditional on selected scenarios/jobs), uploads artifacts, and surfaces results in the PR’s Vitest scenario table.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 19, 2026
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new live Vitest e2e test (agent-turn-latency.test.ts) that measures agent turn latency for OpenClaw and Hermes sandboxes, including retry logic, latency assertions, and JSON artifact output. A corresponding CI workflow job (agent-turn-latency-vitest) is added and wired into the report-to-pr aggregation.

Changes

Agent Turn Latency E2E Scenario

Layer / File(s) Summary
Test configuration and environment setup
test/e2e-scenario/live/agent-turn-latency.test.ts
Defines repo/CLI paths, sandbox/model/provider/timeout constants from environment, and an env() helper that builds per-sandbox process environments with agent selection and inference API key injection.
Test utilities and validation helpers
test/e2e-scenario/live/agent-turn-latency.test.ts
Implements test utilities (bestEffort(), responseBodyAndStatus(), chatContent(), msSince() for latency measurement), configuration validators (assertOpenClawConfig(), assertHermesConfig()), and installSandbox() with bounded retries and transient-failure detection.
Gated live scenario: install, execute, assert, and record latency
test/e2e-scenario/live/agent-turn-latency.test.ts
Implements the full gated live test: orchestrates cleanup, docker check, sequential OpenClaw and Hermes sandbox install/run, per-turn latency measurement via process.hrtime.bigint(), integer-42 and timeout assertions, and writes turn-latency-results.json and legacy-path artifacts.
CI workflow job and PR report wiring
.github/workflows/e2e-vitest-scenarios.yaml
Adds the agent-turn-latency-vitest job (OpenShell install, NVIDIA_INFERENCE_API_KEY injection, Vitest execution, artifact upload) and registers it in the report-to-pr needs array.

Sequence Diagram(s)

sequenceDiagram
  participant CI as GitHub Actions
  participant Test as agent-turn-latency.test.ts
  participant OpenClaw as OpenClaw Sandbox
  participant Hermes as Hermes Sandbox
  participant Artifacts as e2e-artifacts/

  CI->>Test: vitest run (with NVIDIA_INFERENCE_API_KEY)
  Test->>Test: shouldRunLiveE2EScenarios() gate
  Test->>OpenClaw: installSandbox() — install.sh with retries
  OpenClaw-->>Test: install success
  Test->>OpenClaw: run agent via sandbox shell (record bigint start)
  OpenClaw-->>Test: response text
  Test->>Test: assert contains 42 and within latency max
  Test->>OpenClaw: destroy sandbox
  Test->>Hermes: installSandbox() — install.sh with retries
  Hermes-->>Test: install success
  Test->>Hermes: curl /v1/chat/completions (record bigint start)
  Hermes-->>Test: JSON response
  Test->>Test: chatContent() extract, assert 42 and latency
  Test->>Artifacts: write turn-latency-results.json
  CI->>CI: upload artifacts from e2e-artifacts/vitest/agent-turn-latency/
  CI->>CI: report-to-pr includes agent-turn-latency-vitest result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5349: Adds another free-standing live Vitest job to the same e2e-vitest-scenarios.yaml workflow and extends report-to-pr.needs with the same pattern.
  • NVIDIA/NemoClaw#5493: Follows the same pattern — new free-standing live Vitest job in e2e-vitest-scenarios.yaml plus report-to-pr needs update — for a different scenario.
  • NVIDIA/NemoClaw#5494: Adds a standalone live Vitest job to the same workflow and wires it into report-to-pr for PR result aggregation.

Suggested labels

area: e2e, chore

Poem

🐇 Hop hop, the latency clock ticks away,
OpenClaw and Hermes both tested today.
Install, retry, assert — does 42 appear?
The bigint timer marks each millisecond clear.
CI uploads the proof, the PR table glows,
A bunny checks the workflow — and off it goes! 🚀

🚥 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 PR title 'test(e2e): migrate agent turn latency to vitest' directly and concisely describes the main change: migrating an E2E test from shell script to Vitest.
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
  • Commit unit tests in branch e2e-migrate/test-agent-turn-latency

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

@github-code-quality

github-code-quality Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the e2e-migrate/test-age... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main e2e-migrate/test-age... cc5124e +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the e2e-migrate/test-age... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main e2e-migrate/test-age... cc5124e +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

Updated June 20, 2026 23:22 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: agent-turn-latency-vitest

Dispatch hint: agent-turn-latency-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None.

Optional E2E

  • agent-turn-latency-vitest (high): Useful to validate the newly added workflow job and live Vitest scenario. It directly covers the changed test files and verifies the new workflow selector, OpenShell install step, hosted inference secret wiring, OpenClaw turn, and Hermes turn behavior.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: agent-turn-latency-vitest

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: agent-turn-latency-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=agent-turn-latency-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • agent-turn-latency-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/agent-turn-latency.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=agent-turn-latency-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/live/agent-turn-latency-helpers.ts
  • test/e2e-scenario/live/agent-turn-latency.test.ts

Comment thread test/e2e-scenario/live/agent-turn-latency.test.ts Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@test/e2e-scenario/live/agent-turn-latency.test.ts`:
- Around line 157-165: The openclawConfig variable captures raw config output
without redacting sensitive credentials, creating a potential API key leakage
risk into artifacts. The Hermes config path already implements redaction of
apiKey values, but openclawConfig does not. Add similar redaction logic to
openclawConfig to remove or mask sensitive fields like apiKey before the output
is captured in the artifact, following the same redaction pattern that is
already applied to the Hermes config to prevent credentials from leaking into
logs and artifacts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e9e10c9c-b55e-4d56-96da-a59a80764c50

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4d1f5 and 681deec.

📒 Files selected for processing (2)
  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/live/agent-turn-latency.test.ts

Comment thread test/e2e-scenario/live/agent-turn-latency.test.ts Outdated
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: cleanupTurnSandboxes() best-effort cleanup: 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: bestEffort() catches all errors; cleanupTurnSandboxes() wraps destroy, delete, forward stop, and gateway destroy.
  • Source-of-truth review needed: OpenClaw JSON assistant-text tolerance: 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: extractOpenClawAgentText() scans objects until text is found, but collectAssistantText() recurses through message/messages and returns any non-empty string.
  • New secret-bearing job is not locked by a dedicated workflow-boundary validator (.github/workflows/e2e-vitest-scenarios.yaml:745): The new agent-turn-latency-vitest job runs installer, OpenShell, sandbox lifecycle, OpenClaw/Hermes, and hosted inference with NVIDIA_INFERENCE_API_KEY. The job follows several good patterns, but nearby trusted workflow-boundary code has dedicated validators for comparable jobs such as cloud-inference-vitest and no equivalent agent-turn-latency validator was added. That leaves future edits to this secret-bearing job's action pinning, checkout credential handling, secret scope, artifact path/settings, OpenShell discovery, and report-to-pr wiring less protected.
    • Recommendation: Add a validateAgentTurnLatencyVitestJob-style check in tools/e2e-scenarios/workflow-boundary.mts and support-test expectations covering the exact env, secret scoping, pinned actions, checkout persist-credentials=false, npm ci --ignore-scripts, OpenShell install/path validation, artifact settings, timeout, selector metadata, and report-to-pr needs entry.
    • Evidence: The workflow adds agent-turn-latency-vitest with NVIDIA_INFERENCE_API_KEY in the Vitest step. tools/e2e-scenarios/workflow-boundary.mts contains validateCloudInferenceVitestJob and many comparable validators, but grep/read found no agent-turn-latency-specific validator or call.
  • Hosted-compatible inference defaults do not match the retained shell contract (test/e2e-scenario/live/agent-turn-latency-helpers.ts:27): The retained shell source configures compatible hosted inference through ci-compatible-inference.sh before deriving the model, provider, and expected route. The new helper only enables compatible hosted mode when NEMOCLAW_E2E_USE_HOSTED_INFERENCE is exactly "1", so it misses the retained auto-detection for custom/non-NVIDIA providers with a non-nvapi NVIDIA_INFERENCE_API_KEY and does not mirror the NEMOCLAW_CLOUD_EXPERIMENTAL_MODEL fallback. The non-compatible default model also changed from the retained shell's nvidia/nemotron-3-ultra-550b-a55b to nvidia/nemotron-3-super-120b-a12b without an in-diff contract explaining the change.
    • Recommendation: Port or reuse the retained ci-compatible-inference.sh decision logic exactly, or document and test the intentional contract change. Include provider/key-shape auto-detection, endpoint URL, COMPATIBLE_API_KEY, NEMOCLAW_COMPAT_MODEL, NEMOCLAW_PREFERRED_API, NEMOCLAW_CLOUD_EXPERIMENTAL_MODEL fallback, expected route derivation, and the chosen default model.
    • Evidence: New USE_COMPATIBLE_HOSTED is process.env.NEMOCLAW_E2E_USE_HOSTED_INFERENCE === "1" and MODEL falls back to DEFAULT_NVIDIA_MODEL/DEFAULT_COMPAT_MODEL. Retained test/e2e/test-agent-turn-latency-e2e.sh calls nemoclaw_e2e_configure_compatible_inference and derives TURN_MODEL via nemoclaw_e2e_hosted_inference_model or nvidia/nemotron-3-ultra-550b-a55b.
  • OpenClaw JSON text extraction can accept metadata-only strings as the assistant reply (test/e2e-scenario/live/agent-turn-latency-helpers.ts:122): The updated parser now scans multiple JSON objects, which addresses the prior first-object-only issue, but its recursive key list is broader than the retained parser. It treats strings under keys such as message/messages as assistant text, so wrapper or metadata JSON containing "42" in those fields could satisfy the assertion before the actual assistant envelope is parsed.
    • Recommendation: Match the retained openclaw-json.sh extraction contract more closely: collect only text/content/reasoning_content and known assistant containers, and add focused tests for metadata JSON before a valid assistant envelope and metadata-only message/messages values containing 42.
    • Evidence: collectAssistantText() returns any non-empty string and recurses through message/messages. Retained test/e2e/lib/openclaw-json.sh comments say it intentionally ignores metadata fields so IDs, durations, session names, and model/provider details cannot satisfy reply assertions.
  • Hermes API request still relies on ad hoc shell-string payload quoting (test/e2e-scenario/live/agent-turn-latency-helpers.ts:354): hermesTurnCommand() builds a sh -lc script and interpolates a JSON payload into curl -d using manual single-quote replacement. The current payload is locally generated, but MODEL is environment-controlled, making this a security-sensitive shell boundary. The escaping may be safe for common cases, but the PR does not lock the contract with focused injection/corruption tests.
    • Recommendation: Prefer passing the JSON through stdin, a temporary file, or a structured exec argument path instead of embedding it in a shell string. If the shell string remains, add tests proving quotes, semicolons, dollars, backticks, and newlines in model values cannot execute commands or alter the JSON payload.
    • Evidence: hermesTurnCommand(payload) returns a single shell script containing -d '${payload.replace(/'/gu, `'\''`)}'; payload includes MODEL derived from process.env.
  • Cleanup failures are swallowed without a locked source-of-truth contract (test/e2e-scenario/live/agent-turn-latency-helpers.ts:73): Best-effort cleanup is reasonable for absent resources, but this helper catches all cleanup errors around sandbox destroy/delete, forward stop, and gateway destroy. That also hides permission failures, CLI failures, or stale gateway state that could affect the next install or retry. The diff does not identify the exact invalid states being tolerated or prove post-clean state.
    • Recommendation: Document the expected cleanup failure states and either make cleanup idempotent at the source or verify the post-clean state before installs/retries. Add support coverage showing absent resources are tolerated while real cleanup failures or stale gateway/forward/sandbox state cannot silently influence the scenario.
    • Evidence: cleanupTurnSandboxes() wraps destroy/delete/forward stop/gateway destroy in bestEffort(); bestEffort() catches every thrown error and only logs best-effort cleanup failed.
  • Exhausted transient provider validation no longer matches retained skip semantics (test/e2e-scenario/live/agent-turn-latency-helpers.ts:212): The retained shell classifies repeated transient provider validation failures as skip-equivalent after the configured attempts. The Vitest helper retries transient failures, but the test always asserts the final install exitCode is zero. That can turn external provider flakiness into a hard scenario failure and changes the migrated contract without a focused test or documentation.
    • Recommendation: Decide whether exhausted transient provider validation should remain skip-equivalent or become a hard failure, then encode that contract in installSandbox() and a focused support test. Keep cleanup-before-retry behavior covered as part of the same contract.
    • Evidence: installSandbox() retries when isTransientProviderValidationFailure(install) and then returns the last result; agent-turn-latency.test.ts asserts openclawInstall.exitCode and hermesInstall.exitCode are 0. Retained run_install() calls skip after TURN_INSTALL_ATTEMPTS transient provider validation attempts.
  • Focused support coverage is missing for the new helper and secret-artifact branches (test/e2e-scenario/live/agent-turn-latency.test.ts:1): The PR adds a live scenario and secret-bearing workflow job, but the changed behavior depends on many branches that are hard to exercise deterministically in one live run: workflow trusted-code boundaries, hosted-compatible env derivation, JSON envelope compatibility, transient retry cleanup, shell payload quoting, and artifact redaction. Existing support tests cover the generic workflow boundary, but this diff does not add agent-turn-latency-specific helper or boundary tests.
    • Recommendation: Keep the live test as real-system coverage, but add focused support tests for the new helper contracts and security-sensitive artifact behavior.
    • Evidence: New helper paths include positiveInt(), parseJsonObjectAt()/collectAssistantText(), bestEffort(), transient install retry handling, waitHermesHealth(), openclawConfigCommand(), and hermesTurnCommand(); the diff adds only the live test and workflow wiring.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — validateAgentTurnLatencyVitestJob locks NVIDIA_INFERENCE_API_KEY to only the Vitest run step and rejects secret exposure in other steps. This PR changes workflow, installer, OpenShell, sandbox lifecycle, hosted inference, and secret-bearing artifact paths. The live Vitest scenario is appropriate for real-system validation, but focused support tests are still needed for deterministic helper/security contracts that one live run may not cover.
  • **Runtime validation** — validateAgentTurnLatencyVitestJob requires pinned actions, checkout persist-credentials=false, npm ci --ignore-scripts, OpenShell path validation, artifact settings, timeout, selector metadata, and report-to-pr needs wiring. This PR changes workflow, installer, OpenShell, sandbox lifecycle, hosted inference, and secret-bearing artifact paths. The live Vitest scenario is appropriate for real-system validation, but focused support tests are still needed for deterministic helper/security contracts that one live run may not cover.
  • **Runtime validation** — dispatch selector maps scenario agent-turn-latency to agent-turn-latency-vitest and rejects malformed selector input. This PR changes workflow, installer, OpenShell, sandbox lifecycle, hosted inference, and secret-bearing artifact paths. The live Vitest scenario is appropriate for real-system validation, but focused support tests are still needed for deterministic helper/security contracts that one live run may not cover.
  • **Runtime validation** — compatible inference env mapping matches ci-compatible-inference.sh when NEMOCLAW_E2E_USE_HOSTED_INFERENCE=1. This PR changes workflow, installer, OpenShell, sandbox lifecycle, hosted inference, and secret-bearing artifact paths. The live Vitest scenario is appropriate for real-system validation, but focused support tests are still needed for deterministic helper/security contracts that one live run may not cover.
  • **Runtime validation** — compatible inference auto-detects custom provider with a non-nvapi NVIDIA_INFERENCE_API_KEY like the retained shell. This PR changes workflow, installer, OpenShell, sandbox lifecycle, hosted inference, and secret-bearing artifact paths. The live Vitest scenario is appropriate for real-system validation, but focused support tests are still needed for deterministic helper/security contracts that one live run may not cover.
  • **Focused support coverage is missing for the new helper and secret-artifact branches** — Keep the live test as real-system coverage, but add focused support tests for the new helper contracts and security-sensitive artifact behavior.
  • **Acceptance clause:** Refs Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 — add test evidence or identify existing coverage. The deterministic validation context reported linkedIssues: [], so no issue Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 body or comments were available to extract literal acceptance clauses. Repo docs and existing workflow comments indicate Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 governs direct legacy bash-suite migration and Phase 11 retirement, but the issue text itself was not available in this review context.
  • **Acceptance clause:** The new test onboards OpenClaw and Hermes against hosted inference, verifies their managed inference.local configuration, and times one real model-backed turn through each runtime. — add test evidence or identify existing coverage. The live test performs those actions, but parity concerns remain around compatible hosted inference detection/defaults and transient provider validation semantics versus the retained shell source.
Since last review details

Current findings:

  • Source-of-truth review needed: cleanupTurnSandboxes() best-effort cleanup: 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: bestEffort() catches all errors; cleanupTurnSandboxes() wraps destroy, delete, forward stop, and gateway destroy.
  • Source-of-truth review needed: OpenClaw JSON assistant-text tolerance: 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: extractOpenClawAgentText() scans objects until text is found, but collectAssistantText() recurses through message/messages and returns any non-empty string.
  • New secret-bearing job is not locked by a dedicated workflow-boundary validator (.github/workflows/e2e-vitest-scenarios.yaml:745): The new agent-turn-latency-vitest job runs installer, OpenShell, sandbox lifecycle, OpenClaw/Hermes, and hosted inference with NVIDIA_INFERENCE_API_KEY. The job follows several good patterns, but nearby trusted workflow-boundary code has dedicated validators for comparable jobs such as cloud-inference-vitest and no equivalent agent-turn-latency validator was added. That leaves future edits to this secret-bearing job's action pinning, checkout credential handling, secret scope, artifact path/settings, OpenShell discovery, and report-to-pr wiring less protected.
    • Recommendation: Add a validateAgentTurnLatencyVitestJob-style check in tools/e2e-scenarios/workflow-boundary.mts and support-test expectations covering the exact env, secret scoping, pinned actions, checkout persist-credentials=false, npm ci --ignore-scripts, OpenShell install/path validation, artifact settings, timeout, selector metadata, and report-to-pr needs entry.
    • Evidence: The workflow adds agent-turn-latency-vitest with NVIDIA_INFERENCE_API_KEY in the Vitest step. tools/e2e-scenarios/workflow-boundary.mts contains validateCloudInferenceVitestJob and many comparable validators, but grep/read found no agent-turn-latency-specific validator or call.
  • Hosted-compatible inference defaults do not match the retained shell contract (test/e2e-scenario/live/agent-turn-latency-helpers.ts:27): The retained shell source configures compatible hosted inference through ci-compatible-inference.sh before deriving the model, provider, and expected route. The new helper only enables compatible hosted mode when NEMOCLAW_E2E_USE_HOSTED_INFERENCE is exactly "1", so it misses the retained auto-detection for custom/non-NVIDIA providers with a non-nvapi NVIDIA_INFERENCE_API_KEY and does not mirror the NEMOCLAW_CLOUD_EXPERIMENTAL_MODEL fallback. The non-compatible default model also changed from the retained shell's nvidia/nemotron-3-ultra-550b-a55b to nvidia/nemotron-3-super-120b-a12b without an in-diff contract explaining the change.
    • Recommendation: Port or reuse the retained ci-compatible-inference.sh decision logic exactly, or document and test the intentional contract change. Include provider/key-shape auto-detection, endpoint URL, COMPATIBLE_API_KEY, NEMOCLAW_COMPAT_MODEL, NEMOCLAW_PREFERRED_API, NEMOCLAW_CLOUD_EXPERIMENTAL_MODEL fallback, expected route derivation, and the chosen default model.
    • Evidence: New USE_COMPATIBLE_HOSTED is process.env.NEMOCLAW_E2E_USE_HOSTED_INFERENCE === "1" and MODEL falls back to DEFAULT_NVIDIA_MODEL/DEFAULT_COMPAT_MODEL. Retained test/e2e/test-agent-turn-latency-e2e.sh calls nemoclaw_e2e_configure_compatible_inference and derives TURN_MODEL via nemoclaw_e2e_hosted_inference_model or nvidia/nemotron-3-ultra-550b-a55b.
  • OpenClaw JSON text extraction can accept metadata-only strings as the assistant reply (test/e2e-scenario/live/agent-turn-latency-helpers.ts:122): The updated parser now scans multiple JSON objects, which addresses the prior first-object-only issue, but its recursive key list is broader than the retained parser. It treats strings under keys such as message/messages as assistant text, so wrapper or metadata JSON containing "42" in those fields could satisfy the assertion before the actual assistant envelope is parsed.
    • Recommendation: Match the retained openclaw-json.sh extraction contract more closely: collect only text/content/reasoning_content and known assistant containers, and add focused tests for metadata JSON before a valid assistant envelope and metadata-only message/messages values containing 42.
    • Evidence: collectAssistantText() returns any non-empty string and recurses through message/messages. Retained test/e2e/lib/openclaw-json.sh comments say it intentionally ignores metadata fields so IDs, durations, session names, and model/provider details cannot satisfy reply assertions.
  • Hermes API request still relies on ad hoc shell-string payload quoting (test/e2e-scenario/live/agent-turn-latency-helpers.ts:354): hermesTurnCommand() builds a sh -lc script and interpolates a JSON payload into curl -d using manual single-quote replacement. The current payload is locally generated, but MODEL is environment-controlled, making this a security-sensitive shell boundary. The escaping may be safe for common cases, but the PR does not lock the contract with focused injection/corruption tests.
    • Recommendation: Prefer passing the JSON through stdin, a temporary file, or a structured exec argument path instead of embedding it in a shell string. If the shell string remains, add tests proving quotes, semicolons, dollars, backticks, and newlines in model values cannot execute commands or alter the JSON payload.
    • Evidence: hermesTurnCommand(payload) returns a single shell script containing -d '${payload.replace(/'/gu, `'\''`)}'; payload includes MODEL derived from process.env.
  • Cleanup failures are swallowed without a locked source-of-truth contract (test/e2e-scenario/live/agent-turn-latency-helpers.ts:73): Best-effort cleanup is reasonable for absent resources, but this helper catches all cleanup errors around sandbox destroy/delete, forward stop, and gateway destroy. That also hides permission failures, CLI failures, or stale gateway state that could affect the next install or retry. The diff does not identify the exact invalid states being tolerated or prove post-clean state.
    • Recommendation: Document the expected cleanup failure states and either make cleanup idempotent at the source or verify the post-clean state before installs/retries. Add support coverage showing absent resources are tolerated while real cleanup failures or stale gateway/forward/sandbox state cannot silently influence the scenario.
    • Evidence: cleanupTurnSandboxes() wraps destroy/delete/forward stop/gateway destroy in bestEffort(); bestEffort() catches every thrown error and only logs best-effort cleanup failed.
  • Exhausted transient provider validation no longer matches retained skip semantics (test/e2e-scenario/live/agent-turn-latency-helpers.ts:212): The retained shell classifies repeated transient provider validation failures as skip-equivalent after the configured attempts. The Vitest helper retries transient failures, but the test always asserts the final install exitCode is zero. That can turn external provider flakiness into a hard scenario failure and changes the migrated contract without a focused test or documentation.
    • Recommendation: Decide whether exhausted transient provider validation should remain skip-equivalent or become a hard failure, then encode that contract in installSandbox() and a focused support test. Keep cleanup-before-retry behavior covered as part of the same contract.
    • Evidence: installSandbox() retries when isTransientProviderValidationFailure(install) and then returns the last result; agent-turn-latency.test.ts asserts openclawInstall.exitCode and hermesInstall.exitCode are 0. Retained run_install() calls skip after TURN_INSTALL_ATTEMPTS transient provider validation attempts.
  • Focused support coverage is missing for the new helper and secret-artifact branches (test/e2e-scenario/live/agent-turn-latency.test.ts:1): The PR adds a live scenario and secret-bearing workflow job, but the changed behavior depends on many branches that are hard to exercise deterministically in one live run: workflow trusted-code boundaries, hosted-compatible env derivation, JSON envelope compatibility, transient retry cleanup, shell payload quoting, and artifact redaction. Existing support tests cover the generic workflow boundary, but this diff does not add agent-turn-latency-specific helper or boundary tests.
    • Recommendation: Keep the live test as real-system coverage, but add focused support tests for the new helper contracts and security-sensitive artifact behavior.
    • Evidence: New helper paths include positiveInt(), parseJsonObjectAt()/collectAssistantText(), bestEffort(), transient install retry handling, waitHermesHealth(), openclawConfigCommand(), and hermesTurnCommand(); the diff adds only the live test and workflow wiring.

Workflow run details

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

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the v0.0.66 label Jun 19, 2026
@cv cv linked an issue Jun 19, 2026 that may be closed by this pull request
79 tasks
cv added 3 commits June 19, 2026 14:07
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…t-turn-latency

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv added 7 commits June 19, 2026 14:44
…t-turn-latency

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…ncy' into e2e-migrate/test-agent-turn-latency

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Comment thread test/e2e-scenario/live/agent-turn-latency-helpers.ts Fixed
Comment thread test/e2e-scenario/live/agent-turn-latency-helpers.ts Fixed
cv added 2 commits June 20, 2026 11:34
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…t-turn-latency

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

cv commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

Resolved the PR #5554 merge conflicts and addressed several still-valid review items.

Updates pushed:

  • Merged latest origin/main into e2e-migrate/test-agent-turn-latency and resolved the workflow conflict by keeping both free-standing jobs: agent-turn-latency-vitest and hermes-inference-switch-vitest, both wired into report-to-pr.needs.
  • Hardened agent-turn-latency-vitest OpenShell discovery to fail fast when openshell is unavailable after install, and to print openshell --version before running Vitest.
  • Avoided writing the Hermes API_SERVER_KEY into an artifacted probe by removing the separate key retrieval command; the Hermes turn command now sources and uses the key entirely inside the same sandbox command.
  • Changed OpenClaw config capture to emit a redacted JSON view for token/key/secret-like fields before artifact capture.
  • Pre-cleans turn-latency sandboxes before the first install attempt, not only before retries.
  • Made OpenClaw JSON extraction continue scanning parseable JSON objects until one yields assistant text.
  • Labeled best-effort cleanup warnings so suppressed cleanup failures are visible and attributable in logs.

Validation rerun:

npx biome check --write .github/workflows/e2e-vitest-scenarios.yaml test/e2e-scenario/live/agent-turn-latency.test.ts test/e2e-scenario/live/agent-turn-latency-helpers.ts test/e2e-scenario/live/hermes-inference-switch.test.ts test/e2e-scenario/live/hermes-inference-switch-helpers.ts
npm run typecheck:cli
npx vitest run test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/e2e-live-project-config.test.ts

Pushed signed merge/update commits over SSH because the default HTTPS token cannot update workflow files.

…t-turn-latency

# Conflicts:
#	.github/workflows/e2e-vitest-scenarios.yaml
@cv

cv commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

Merged latest origin/main again after #5555 landed and resolved the workflow conflict by keeping all free-standing jobs wired into report-to-pr.needs: agent-turn-latency-vitest, kimi-inference-compat-vitest, and hermes-inference-switch-vitest.

Validation rerun after this second conflict resolution:

npx biome check --write .github/workflows/e2e-vitest-scenarios.yaml test/e2e-scenario/live/agent-turn-latency.test.ts test/e2e-scenario/live/agent-turn-latency-helpers.ts test/e2e-scenario/live/hermes-inference-switch.test.ts test/e2e-scenario/live/hermes-inference-switch-helpers.ts test/e2e-scenario/live/kimi-inference-compat.test.ts test/e2e-scenario/live/kimi-inference-compat-helpers.ts
npm run typecheck:cli
npx vitest run test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/e2e-live-project-config.test.ts

@cv
cv merged commit 7691355 into main Jun 21, 2026
40 checks passed
@cv
cv deleted the e2e-migrate/test-agent-turn-latency branch June 21, 2026 21:02
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants