Skip to content

test(e2e): add inference runtime helpers#5056

Merged
cv merged 21 commits into
mainfrom
codex/e2e-fanout-05-inference-runtime-helpers
Jun 10, 2026
Merged

test(e2e): add inference runtime helpers#5056
cv merged 21 commits into
mainfrom
codex/e2e-fanout-05-inference-runtime-helpers

Conversation

@cv

@cv cv commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the typed inference runtime helper surface for the Vitest E2E scenario runner.

Related Issue

Refs #4941
Refs #4990
Refs #4349
Depends on #5046, #5052, and the shared runtime-suite base stack.
Stacked on branch codex/e2e-fanout-01-inventory-internals.

Changes

  • Added RuntimePhaseFixture and the runtime Vitest fixture for inference runtime probes.
  • Added reusable helpers for sandbox-side inference.local models, chat completion, and HTTP status checks.
  • Added trusted-provider compatible endpoint helpers for models/chat probes while preserving shell-probe artifact capture and redaction.
  • Validate model-list responses for OpenAI-style { data: [...] } and Ollama-style { models: [...] } payloads so readiness helpers cannot pass on {} or error-only JSON.
  • Auto-redact sensitive custom header values and honor provider curlMaxTimeSeconds as curl --max-time.
  • Extended ProviderClient with a request-level JSON API that returns both parsed JSON and the captured ShellProbeResult.
  • Added framework tests for route normalization, argv construction, redaction values, provider-compatible requests, model-list validation, provider curl timeout propagation, and malformed response handling.

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)

Verified locally:

  • npx vitest run --project e2e-scenario-framework test/e2e-scenario/framework-tests/e2e-phase-runtime.test.ts test/e2e-scenario/framework-tests/e2e-clients.test.ts --silent=false --reporter=default
  • npx vitest run --project e2e-scenario-framework --silent=false --reporter=default
  • npm run typecheck:cli
  • npx prek run --files test/e2e-scenario/framework/clients/provider.ts test/e2e-scenario/framework/clients/index.ts test/e2e-scenario/framework/e2e-test.ts test/e2e-scenario/framework/phases/index.ts test/e2e-scenario/framework/phases/runtime.ts test/e2e-scenario/framework-tests/e2e-phase-runtime.test.ts --skip test-cli
  • git diff --check

CI/advisor evidence:

  • Required PR checks are green on the PR head.
  • PR review advisor: 0 needs attention, 0 worth checking, 0 nice ideas.
  • E2E recommendation advisor: no product E2E required.
  • E2E scenario advisor requested e2e-scenarios-all; dispatched run https://github.com/NVIDIA/NemoClaw/actions/runs/27241683412. The relevant ubuntu-repo-cloud-openclaw scenario passed. The all-run is red due to pre-existing scenario-runner coverage gaps outside this PR's helper surface, including generated scenarios whose onboarding profile ids are not yet implemented by test/e2e-scenario/nemoclaw_scenarios/onboard/dispatch.sh (for example openai-compatible-openclaw, cloud-nvidia-openclaw-resume-after-interrupt) and a Hermes-specific runtime.hermes.history-writable assertion that fails after onboarding/inference pass because it cannot determine shield state.

Note: the full pre-commit hook's test-cli step still fails locally in test/release-latest-tag.test.ts because this machine's global Git config enables SSH commit signing but the private signing key is unavailable. The focused E2E framework suite and CLI typecheck pass.


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

jyaunches and others added 7 commits June 9, 2026 12:24
`liveScenarioSupport` previously rejected any scenario that declared an
`environment.lifecycle`, so post-onboard host mutations (reboot, rebuild,
upgrade, drift) could not surface in the live Vitest matrix at all.

Replace the unconditional reject with a `SUPPORTED_LIFECYCLES` whitelist
that starts with the single profile the upcoming post-reboot-recovery
fixture dispatches: `post-reboot-recovery`. Future profiles must land the
dispatcher branch and an expected-state in the same change set, so the
whitelist stays in lockstep with what the runner can actually execute.

Prepares the runner for #4423's failing-test-first guard, which needs a
post-reboot lifecycle scenario to demonstrate registry preservation +
Docker-backed sandbox recovery on Linux/Spark Docker-driver hosts.

Refs #4423
Adds two host-side state-validation probes the live runner needs to
express the regression target tracked by #4423:

  * `local-registry-entry-present` reads `~/.nemoclaw/sandboxes.json`
    and asserts the scenario's sandbox name is still recorded. This is
    deliberately orthogonal to `sandbox.expected`: post-reboot bugs
    can wipe the local registry while the live OpenShell gateway is
    healthy, and only a host-side probe catches the data-loss
    regression.

  * `docker-sandbox-container-present` runs
    `docker ps -a --filter label=openshell.ai/sandbox-name=<name>` and
    accepts running, stopped, or `*-nemoclaw-gpu-backup-*` sibling
    containers. The label filter mirrors `OPENSHELL_SANDBOX_NAME_LABEL`
    used by `findOpenShellDockerSandboxContainerIds` in
    `src/lib/onboard/docker-gpu-patch.ts`, so the probe stays in lock-
    step with how OpenShell labels containers today.

Probe wiring:

  * `StateProbeId` extended with the two new probe ids.
  * `ExpectedState` gains `localRegistry` and `dockerSandboxContainer`
    optional dimensions; `probesForState` emits the new probes only
    for `expected: "present"`. Negative-direction probes are
    intentionally omitted today and pinned by a probesForState test.
  * `StateValidationPhaseFixture.from()` now accepts either an
    expected-state ID or an inline `ExpectedState`, so unit tests can
    drive new probes without registering synthetic states in the
    typed registry. The live runner still calls `from(id, instance)`.
  * Fixture takes an optional `ProbeIO` injection so tests can stub
    the registry reader without touching `~/.nemoclaw`.

No callers of the existing typed registry are affected: every shipped
expected-state leaves `localRegistry` and `dockerSandboxContainer`
unset, so `probesForState` returns the same probe lists as before.

Refs #4423
Adds a Vitest phase fixture that mutates host state between onboarding
and state-validation, so live scenarios can express post-onboard
invariants the legacy bash runner has no equivalent for.

`LifecyclePhaseFixture.simulate("post-reboot-recovery", instance, opts)`
reproduces the host-side conditions of a DGX Spark / Linux Docker-driver
reboot in two modes:

  * `stop-original` (default)   — `openshell gateway stop` + `docker
                                   stop` of the labeled sandbox
                                   container. Models the common reboot
                                   outcome where OpenShell forgets the
                                   sandbox while Docker keeps the
                                   container exited but labeled.

  * `rename-to-gpu-backup`      — additionally `docker rename`s the
                                   container to a `*-nemoclaw-gpu-
                                   backup-<ts>` sibling, mirroring the
                                   GPU-patch reboot path in
                                   `src/lib/onboard/docker-gpu-patch.ts`.

Both modes register cleanups (in reverse order) to restore the
container so test teardown leaves Docker in a usable state.

Wiring:

  * `framework/phases/index.ts` re-exports the fixture and types.
  * `framework/e2e-test.ts` registers a `lifecycle` Vitest fixture on
    `E2EScenarioFixtures`, wired with the shared `host`, `sandbox`,
    and `cleanup` registries.
  * `live/registry-scenarios.test.ts` invokes
    `lifecycle.simulate(profile, instance)` between `onboard.from(...)`
    and `stateValidation.from(...)` whenever the scenario declares a
    whitelisted `environment.lifecycle`. Scenarios that omit lifecycle
    are unaffected. A scenario whose lifecycle is whitelisted by
    `runtime-support.ts` but NOT dispatched by the fixture fails fast
    with a clear error so the whitelist and dispatcher stay in lock-
    step.

Coverage in `e2e-phase-lifecycle.test.ts` exercises both modes,
gateway-stop tolerance, the no-labeled-container failure case, the
docker-discover failure case, the unsupported-profile rejection,
the cleanup queue order, and `buildBackupContainerName` truncation.

The fixture is intentionally narrow on profiles: only
`post-reboot-recovery` is dispatched today. Adding rebuild, upgrade,
or drift profiles is a separate, equally narrow change set that must
land the dispatcher branch and `SUPPORTED_LIFECYCLES` whitelist
together.

Refs #4423
Registers the failing-test-first guard for #4423 in the typed scenario
registry so the live Vitest matrix from #5006 fans it out as a
dedicated CI job. Builds on the framework primitives added earlier in
this PR (lifecycle phase fixture, host-side probes, lifecycle whitelist).

Additions:

  * `post-reboot-recovery-ready` expected-state in
    `scenarios/expected-states.ts` declaring the user-visible
    invariants that must hold after a `nemoclaw <name> status` call
    on a freshly-rebooted DGX Spark / Linux Docker-driver host:
      - cli installed,
      - gateway healthy (the user-systemd unit from #4580 brings it
        back up before status runs),
      - sandbox running (recovery completed in time),
      - localRegistry entry preserved (the user-visible regression
        target — destroyed on unfixed `main`),
      - dockerSandboxContainer present (recovery didn't delete the
        labeled container or its `*-nemoclaw-gpu-backup-*` sibling).

  * `ubuntu-repo-docker-post-reboot-recovery` scenario in
    `scenarios/scenarios/baseline.ts` wiring
    `ubuntuRepoDockerLifecycle("cloud-openclaw", "post-reboot-recovery")`
    against the new expected-state and a smoke suite. Carries a
    description that explains the RED/GREEN contract and points to the
    PR-A fix landing in `src/lib/`.

  * `manifests/openclaw-nvidia-post-reboot-recovery.yaml` declares
    `lifecycle: post-reboot-recovery` and the same NVIDIA_API_KEY
    credential ref the cloud-openclaw scenarios use.

  * `.github/workflows/e2e-scenarios.yaml` ROUTES table gains the new
    scenario so the workflow-boundary test
    (`e2e-scenarios-workflow.test.ts`) routes every typed id.

Test pinning:

  * `e2e-scenario-matrix.test.ts` updated from a 1-entry to a 2-entry
    live matrix expectation. The new entry asserts on
    `expectedStateId: "post-reboot-recovery-ready"` so a future
    accidental dropped-lifecycle change to the scenario regresses
    loudly.

  * `e2e-live-registry-discovery.test.ts` swaps the synthetic
    whitelist-coverage test for an assertion against the real
    `ubuntu-repo-docker-post-reboot-recovery` registry entry.

Behavior:

  * On unfixed `main`, the live runner's lifecycle phase stops the
    OpenShell gateway runtime and `docker stop`s the labeled sandbox
    container. State-validation then runs `nemoclaw <name> status`
    (which restarts the gateway via systemd) and the destructive
    `missing` branch in `src/lib/actions/sandbox/status.ts` wipes the
    local registry entry. The `local-registry-entry-present` probe
    fails. Scenario goes RED.

  * On the PR-A fix branch, the new Docker-driver sandbox recovery
    helper restarts the labeled container before stale-removal can
    fire, registry survives, all five probes pass. Scenario flips
    GREEN.

The bash-side legacy compiler emits a
`lifecycle.profile.post-reboot-recovery` PhaseAction pointing at
`nemoclaw_scenarios/lifecycle/dispatch.sh`, but the legacy bash worker
is intentionally not provided: this scenario is Vitest-only. The
typed runner's `LifecyclePhaseFixture` handles dispatch directly. If
the legacy runner is invoked against this scenario it errors out at
the dispatcher; that's the right failure mode while the bash side
stays on its own retirement clock.

Refs #4423
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 9, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 9, 2026

Copy link
Copy Markdown

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.

@cv cv added area: e2e End-to-end tests, nightly failures, or validation infrastructure area: inference Inference routing, serving, model selection, or outputs chore Build, CI, dependency, or tooling maintenance labels Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@cv, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 56 minutes and 1 second. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b83c9ccf-54d0-40ec-a124-d13079a155dd

📥 Commits

Reviewing files that changed from the base of the PR and between b3c295a and c624f0d.

📒 Files selected for processing (7)
  • test/e2e-scenario/framework-tests/e2e-clients.test.ts
  • test/e2e-scenario/framework-tests/e2e-phase-runtime.test.ts
  • test/e2e-scenario/framework/clients/index.ts
  • test/e2e-scenario/framework/clients/provider.ts
  • test/e2e-scenario/framework/e2e-test.ts
  • test/e2e-scenario/framework/phases/index.ts
  • test/e2e-scenario/framework/phases/runtime.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/e2e-fanout-05-inference-runtime-helpers

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

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: e2e-vitest-scenarios:ubuntu-repo-cloud-openclaw

Dispatch hint: ubuntu-repo-cloud-openclaw

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No merge-blocking E2E is required because this PR only changes E2E scenario framework/test code and does not modify installer, onboarding, sandbox lifecycle, credentials, network policy, inference routing implementation, deployment code, or real assistant runtime behavior.

Optional E2E

  • e2e-vitest-scenarios:ubuntu-repo-cloud-openclaw (medium): Optional confidence check that the live Vitest scenario harness still imports and runs after adding the runtime fixture to the shared E2E fixture context. Not merge-blocking because the changed runtime fixture is not yet consumed by the live registry scenario flow.

New E2E recommendations

  • runtime-inference-phase (medium): The new RuntimePhaseFixture introduces real inference.local and provider chat/model probes, but existing live scenarios do not appear to dispatch runtime-phase fixture assertions yet. Add a live scenario or runtime suite that uses expectInferenceLocalModels, expectInferenceLocalChatCompletion, and provider endpoint probes after onboarding.
    • Suggested test: Add a live Vitest scenario runtime suite for ubuntu-repo-cloud-openclaw that verifies inference.local /v1/models and /v1/chat/completions through RuntimePhaseFixture.

Dispatch hint

  • Workflow: E2E / Vitest Scenarios
  • jobs input: ubuntu-repo-cloud-openclaw

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

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 framework/runtime fixture and provider client code changed under test/e2e-scenario/framework, including runtime phase support and provider JSON request handling. These are cross-cutting scenario support changes rather than a suite-script-only change, so run the full scenario fan-out.
    • 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-tests/e2e-phase-runtime.test.ts
  • test/e2e-scenario/framework/clients/index.ts
  • test/e2e-scenario/framework/clients/provider.ts
  • test/e2e-scenario/framework/e2e-test.ts
  • test/e2e-scenario/framework/phases/index.ts
  • test/e2e-scenario/framework/phases/runtime.ts

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: Chat-completion response compatibility for message.content and text: 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: `hasChoiceContent` accepts both `message.content` and `text`; the text path is not covered by the added tests.
  • Validate maxTokens before sending chat probes (test/e2e-scenario/framework/phases/runtime.ts:163): The runtime helper validates curlMaxTimeSeconds before command construction, but chat maxTokens is serialized directly into the OpenAI-compatible payload. Values such as NaN or Infinity become null in JSON.stringify, and zero or negative values are sent to the provider, which can make live failures provider-dependent and harder to diagnose.
    • Recommendation: Reject non-finite, non-positive, and preferably non-integer maxTokens before invoking curl, mirroring the existing fail-fast validation for curlMaxTimeSeconds.
    • Evidence: openAiChatPayload builds `max_tokens: options.maxTokens ?? DEFAULT_CHAT_MAX_TOKENS` without validation, while surrounding tests only cover invalid curlMaxTimeSeconds.

🌱 Nice ideas

  • Document or test the completion-style text fallback for chat responses (test/e2e-scenario/framework/phases/runtime.ts:182): The chat-completion shape checker accepts either `message.content` or a completion-style `text` field. If this compatibility path is intentional, it should have explicit evidence so a misrouted or non-chat provider response does not silently become the source of truth for chat readiness.
    • Recommendation: Either remove the `choice.text` fallback for chat-completion probes, or add a focused test and short code comment explaining which compatible provider requires it and when it can be removed.
    • Evidence: `hasChoiceContent` accepts `(choice as { text?: unknown }).text`, but the added tests exercise `message.content` success and malformed JSON failure, not the text fallback.
Consider writing more tests for
Since last review details

Current findings:

  • Source-of-truth review needed: Chat-completion response compatibility for message.content and text: 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: `hasChoiceContent` accepts both `message.content` and `text`; the text path is not covered by the added tests.
  • Validate maxTokens before sending chat probes (test/e2e-scenario/framework/phases/runtime.ts:163): The runtime helper validates curlMaxTimeSeconds before command construction, but chat maxTokens is serialized directly into the OpenAI-compatible payload. Values such as NaN or Infinity become null in JSON.stringify, and zero or negative values are sent to the provider, which can make live failures provider-dependent and harder to diagnose.
    • Recommendation: Reject non-finite, non-positive, and preferably non-integer maxTokens before invoking curl, mirroring the existing fail-fast validation for curlMaxTimeSeconds.
    • Evidence: openAiChatPayload builds `max_tokens: options.maxTokens ?? DEFAULT_CHAT_MAX_TOKENS` without validation, while surrounding tests only cover invalid curlMaxTimeSeconds.
  • Document or test the completion-style text fallback for chat responses (test/e2e-scenario/framework/phases/runtime.ts:182): The chat-completion shape checker accepts either `message.content` or a completion-style `text` field. If this compatibility path is intentional, it should have explicit evidence so a misrouted or non-chat provider response does not silently become the source of truth for chat readiness.
    • Recommendation: Either remove the `choice.text` fallback for chat-completion probes, or add a focused test and short code comment explaining which compatible provider requires it and when it can be removed.
    • Evidence: `hasChoiceContent` accepts `(choice as { text?: unknown }).text`, but the added tests exercise `message.content` success and malformed JSON failure, not the text fallback.

Workflow run details

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

jyaunches and others added 10 commits June 9, 2026 14:22
Prek hook auto-fixed formatting in 6 files added/touched by this PR.
No behavior change.
The biome-format commit accidentally added a node_modules symlink
alongside the formatting fixes. Remove it; the directory is already
in .gitignore.
…nventory-internals

# Conflicts:
#	test/e2e-scenario/framework-tests/e2e-phase-lifecycle.test.ts
#	test/e2e-scenario/framework/phases/lifecycle.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

cv added 2 commits June 9, 2026 15:54
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv marked this pull request as ready for review June 9, 2026 23:20
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv merged commit 7a471a0 into main Jun 10, 2026
30 checks passed
@cv cv deleted the codex/e2e-fanout-05-inference-runtime-helpers branch June 10, 2026 06:00
@cv cv added the v0.0.63 Release target label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure area: inference Inference routing, serving, model selection, or outputs chore Build, CI, dependency, or tooling maintenance v0.0.63 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants