Skip to content

test(e2e): migrate cron preflight to vitest - #5550

Merged
cv merged 2 commits into
mainfrom
e2e-migrate/test-cron-preflight-inference-local
Jun 19, 2026
Merged

test(e2e): migrate cron preflight to vitest#5550
cv merged 2 commits into
mainfrom
e2e-migrate/test-cron-preflight-inference-local

Conversation

@cv

@cv cv commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates test/e2e/test-cron-preflight-inference-local-e2e.sh into a typed live Vitest scenario. The new coverage preserves the runtime contract by onboarding a real OpenClaw sandbox and invoking the in-sandbox cron model-provider preflight helper against the managed inference.local route.

Related Issue

Refs #5098

Changes

  • Add test/e2e-scenario/live/cron-preflight-inference-local.test.ts with typed artifact capture, cleanup, and secret redaction.
  • Wire cron-preflight-inference-local-vitest into .github/workflows/e2e-vitest-scenarios.yaml as a free-standing dispatchable Vitest job.
  • Preserve legacy shell deletion for Phase 11 cleanup per the migration governance in Epic: Migrate legacy bash E2E into the Vitest E2E system #5098.

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/cron-preflight-inference-local.test.ts
  • NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/cron-preflight-inference-local.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/cron-preflight-inference-local.test.ts
  • git diff --check

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

Summary by CodeRabbit

  • Tests
    • Added a new live E2E scenario to validate in-sandbox cron preflight can detect and report availability for the managed inference.local provider.
    • Introduced more robust CI coverage by adding a dedicated Vitest job to run the scenario and upload results for review.

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

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: cb5eb994-ea3b-4589-a389-12babc700ae3

📥 Commits

Reviewing files that changed from the base of the PR and between 916530e and bfb6eb3.

📒 Files selected for processing (1)
  • test/e2e-scenario/live/cron-preflight-inference-local.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e-scenario/live/cron-preflight-inference-local.test.ts

📝 Walkthrough

Walkthrough

A new live Vitest E2E test file is added that validates preflightCronModelProvider against the managed inference.local provider inside a real sandbox. A matching CI workflow job (cron-preflight-inference-local-vitest) is added to .github/workflows/e2e-vitest-scenarios.yaml, and the report-to-pr job is updated to include the new job in its needs list.

Changes

Cron Preflight inference.local E2E Test and CI Wiring

Layer / File(s) Summary
In-sandbox probe script and test helpers
test/e2e-scenario/live/cron-preflight-inference-local.test.ts
Defines module constants, the embedded base64-encoded Node probe script that scans OpenClaw dist directories for the preflight module, dynamically imports it, reads sandbox config, selects the inference.local managed provider, invokes preflightCronModelProvider, and prints structured JSON with exit status. Also defines CronPreflightProbeJson typing, commandEnv, bestEffort, parseProbeJson, probeShell, and cleanupCronSandbox helpers.
Live Vitest scenario body and assertions
test/e2e-scenario/live/cron-preflight-inference-local.test.ts
Implements the skipped live scenario: Docker availability check (throw vs. skip per CI), cleanup hooks for sandbox destruction and OpenShell deletion, install.sh retry/backoff, probe execution via nemoclaw exec, artifact write, and assertions on exit code 0, status === "available", baseUrl === "https://inference.local/v1", and absence of EAI_AGAIN/unreachable messages.
CI workflow job and report-to-pr wiring
.github/workflows/e2e-vitest-scenarios.yaml
Adds the cron-preflight-inference-local-vitest job (env vars, OpenShell install, Vitest run targeting the new test file, artifact upload) and includes it in the report-to-pr job's needs array.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5129: Fixes the same preflightCronModelProvider / inference.local availability behavior that this PR's live scenario asserts against.
  • NVIDIA/NemoClaw#5354: Both PRs add free-standing Vitest jobs to the same .github/workflows/e2e-vitest-scenarios.yaml and update report-to-pr.needs with the new job ID.
  • NVIDIA/NemoClaw#5362: Both PRs add new live Vitest jobs and wire them into the shared report-to-pr.needs list for PR status aggregation.

Suggested labels

area: e2e, chore, v0.0.65

🐰 A sandbox awakes, inference.local in sight,
The preflight probe hops through directories at night,
It reads the config JSON, finds the managed key,
Asserts "available" with a triumphant squee!
No EAI_AGAIN, the cron is set free~ 🎉

🚥 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 accurately summarizes the main change: migrating a cron preflight test from shell script to Vitest, which is the core objective.
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 e2e-migrate/test-cron-preflight-inference-local

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-cro... 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-cro... bfb6eb3 +/-
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-cro... 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-cro... bfb6eb3 +/-
src/lib/state/o...oard-session.ts 90%
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/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 18%

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

@cv cv added the v0.0.66 label Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cron-preflight-inference-local-vitest
Optional E2E: inference-routing-vitest, cloud-inference-vitest

Dispatch hint: cron-preflight-inference-local-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cron-preflight-inference-local-vitest (high): This PR adds the cron preflight inference.local live scenario and its workflow job; the new job should be run to validate the workflow wiring, OpenShell install/setup, sandbox onboarding, credential propagation, and inference.local preflight behavior.

Optional E2E

  • inference-routing-vitest (medium): Adjacent confidence check for inference routing classification and cleanup behavior, but not merge-blocking because the PR only adds a new scenario/job rather than changing runtime routing code.
  • cloud-inference-vitest (high): Adjacent live hosted-inference smoke that exercises install.sh/onboard and sandbox inference.local, useful if reviewers want extra confidence in provider availability and credential handling.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: cron-preflight-inference-local-vitest

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: cron-preflight-inference-local-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=cron-preflight-inference-local-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

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

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/live/cron-preflight-inference-local.test.ts

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: Dynamic discovery of installed OpenClaw preflight module: 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: `findPreflightModule()` scans all `.js/.mjs/.cjs` files below the two dist roots and returns the first body containing `cron-model-provider-preflight` and `preflightCronModelProvider`.
  • Source-of-truth review needed: Tolerant probe JSON parsing: 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: `parseProbeJson()` splits output lines and returns the first trimmed line that starts with `{"providerKey"`; the legacy bash used `grep ... | tail -n 1`.
  • Make probeShell setup failures fail before running node (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:194): The previous nonzero-node cleanup issue is fixed, but the current semicolon placement means the shell proceeds to `node "$__probe"` even if sourcing `/tmp/nemoclaw-proxy-env.sh`, creating the temp file, or decoding the probe failed. That can mask the real setup failure behind a secondary node/file error and weakens the intended fail-fast setup contract.
    • Recommendation: Keep setup commands chained with `&&` through the node invocation, then use a semicolon-only cleanup tail, for example: `. /tmp/... && __probe=... && printf ... > "$__probe" && node "$__probe"; __rc=$?; rm -f "$__probe"; exit "$__rc"`.
    • Evidence: `probeShell()` returns `[setup commands].join(" && ") + '; node "$__probe"; __rc=$?; rm -f "$__probe"; exit "$__rc"'`, so the semicolon separates node execution from setup success.
  • Tighten the fail-closed contract around marker-based OpenClaw module discovery (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:47): The live probe imports the first installed OpenClaw dist file containing both `cron-model-provider-preflight` and `preflightCronModelProvider`. This mirrors the legacy script and is scoped to the sandbox-installed product, but it is validating an SSRF/proxy-sensitive `inference.local` preflight path. If the dist layout changes or another file happens to match those markers, the test could import an unintended module and provide false confidence.
    • Recommendation: Add a fail-closed assertion around the discovered target path/shape, or document in the probe why marker-based discovery is the intended temporary boundary until OpenClaw exposes a stable direct entrypoint. Prefer making the intended runtime entrypoint explicit when one becomes available.
    • Evidence: `findPreflightModule()` recursively scans `/usr/local/lib/node_modules/openclaw/dist` and `/usr/lib/node_modules/openclaw/dist` for `.js/.mjs/.cjs` files whose body includes the marker strings, then dynamically imports the first match.
  • Add negative helper coverage for probe parsing and cleanup behavior (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:184): The new live scenario covers the positive managed `inference.local` path, but does not exercise the helper behavior when the probe setup fails, the node probe exits nonzero, or output contains malformed/noisy/repeated JSON. Those paths are where harness regressions can cause false positives or confusing failures without requiring live provider quota.
    • Recommendation: Add focused support tests for the helper contracts, especially preserving setup failure exit behavior, removing the temporary probe file while preserving node exit code, and selecting the intended structured provider result from noisy output.
    • Evidence: The test asserts the positive path (`parsed` exists, `probe.exitCode` is `0`, `status` is `available`, and `baseUrl` is `https://inference.local/v1\`\), while `parseProbeJson()` simply picks the first line starting with `{"providerKey"` and no negative-path tests cover shell cleanup or parsing drift.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — cron-preflight-inference-local workflow selector selects only cron-preflight-inference-local-vitest and produces an empty registry matrix. The changed behavior crosses workflow selector, installer, Docker/OpenShell sandbox lifecycle, hosted inference credential, and in-sandbox OpenClaw runtime boundaries. Static/unit checks can validate wiring and helper behavior, but cannot prove the live `inference.local` preflight contract by themselves.
  • **Runtime validation** — probeShell preserves the setup failure exit code and does not run node when proxy env or temp-file setup fails. The changed behavior crosses workflow selector, installer, Docker/OpenShell sandbox lifecycle, hosted inference credential, and in-sandbox OpenClaw runtime boundaries. Static/unit checks can validate wiring and helper behavior, but cannot prove the live `inference.local` preflight contract by themselves.
  • **Runtime validation** — probeShell removes the temporary probe file and returns the node exit code when the probe exits nonzero. The changed behavior crosses workflow selector, installer, Docker/OpenShell sandbox lifecycle, hosted inference credential, and in-sandbox OpenClaw runtime boundaries. Static/unit checks can validate wiring and helper behavior, but cannot prove the live `inference.local` preflight contract by themselves.
  • **Runtime validation** — parseProbeJson selects the final structured provider result when stdout/stderr contain noisy or repeated providerKey JSON lines. The changed behavior crosses workflow selector, installer, Docker/OpenShell sandbox lifecycle, hosted inference credential, and in-sandbox OpenClaw runtime boundaries. Static/unit checks can validate wiring and helper behavior, but cannot prove the live `inference.local` preflight contract by themselves.
  • **Runtime validation** — cron preflight live probe records status=available, baseUrl=https://inference.local/v1, no EAI_AGAIN, and no NVIDIA key in artifacts. The changed behavior crosses workflow selector, installer, Docker/OpenShell sandbox lifecycle, hosted inference credential, and in-sandbox OpenClaw runtime boundaries. Static/unit checks can validate wiring and helper behavior, but cannot prove the live `inference.local` preflight contract by themselves.
  • **Add negative helper coverage for probe parsing and cleanup behavior** — Add focused support tests for the helper contracts, especially preserving setup failure exit behavior, removing the temporary probe file while preserving node exit code, and selecting the intended structured provider result from noisy output.
  • **Acceptance clause:** No deterministic linked issue clauses or issue comments were available for this review. The PR body says `Refs Epic: Migrate legacy bash E2E into the Vitest E2E system #5098`, but no linked issue body/comments were supplied in the validation context. — add test evidence or identify existing coverage. Validation context reported `linkedIssues: []`; acceptance was therefore assessed against the diff and nearby migration patterns rather than literal linked-issue clauses.
  • **Dynamic discovery of installed OpenClaw preflight module** — The live test fails if no marker-matching module exists, if the export is missing, or if the imported preflight does not return `available` for the managed provider.. `findPreflightModule()` scans all `.js/.mjs/.cjs` files below the two dist roots and returns the first body containing `cron-model-provider-preflight` and `preflightCronModelProvider`.
Since last review details

Current findings:

  • Source-of-truth review needed: Dynamic discovery of installed OpenClaw preflight module: 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: `findPreflightModule()` scans all `.js/.mjs/.cjs` files below the two dist roots and returns the first body containing `cron-model-provider-preflight` and `preflightCronModelProvider`.
  • Source-of-truth review needed: Tolerant probe JSON parsing: 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: `parseProbeJson()` splits output lines and returns the first trimmed line that starts with `{"providerKey"`; the legacy bash used `grep ... | tail -n 1`.
  • Make probeShell setup failures fail before running node (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:194): The previous nonzero-node cleanup issue is fixed, but the current semicolon placement means the shell proceeds to `node "$__probe"` even if sourcing `/tmp/nemoclaw-proxy-env.sh`, creating the temp file, or decoding the probe failed. That can mask the real setup failure behind a secondary node/file error and weakens the intended fail-fast setup contract.
    • Recommendation: Keep setup commands chained with `&&` through the node invocation, then use a semicolon-only cleanup tail, for example: `. /tmp/... && __probe=... && printf ... > "$__probe" && node "$__probe"; __rc=$?; rm -f "$__probe"; exit "$__rc"`.
    • Evidence: `probeShell()` returns `[setup commands].join(" && ") + '; node "$__probe"; __rc=$?; rm -f "$__probe"; exit "$__rc"'`, so the semicolon separates node execution from setup success.
  • Tighten the fail-closed contract around marker-based OpenClaw module discovery (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:47): The live probe imports the first installed OpenClaw dist file containing both `cron-model-provider-preflight` and `preflightCronModelProvider`. This mirrors the legacy script and is scoped to the sandbox-installed product, but it is validating an SSRF/proxy-sensitive `inference.local` preflight path. If the dist layout changes or another file happens to match those markers, the test could import an unintended module and provide false confidence.
    • Recommendation: Add a fail-closed assertion around the discovered target path/shape, or document in the probe why marker-based discovery is the intended temporary boundary until OpenClaw exposes a stable direct entrypoint. Prefer making the intended runtime entrypoint explicit when one becomes available.
    • Evidence: `findPreflightModule()` recursively scans `/usr/local/lib/node_modules/openclaw/dist` and `/usr/lib/node_modules/openclaw/dist` for `.js/.mjs/.cjs` files whose body includes the marker strings, then dynamically imports the first match.
  • Add negative helper coverage for probe parsing and cleanup behavior (test/e2e-scenario/live/cron-preflight-inference-local.test.ts:184): The new live scenario covers the positive managed `inference.local` path, but does not exercise the helper behavior when the probe setup fails, the node probe exits nonzero, or output contains malformed/noisy/repeated JSON. Those paths are where harness regressions can cause false positives or confusing failures without requiring live provider quota.
    • Recommendation: Add focused support tests for the helper contracts, especially preserving setup failure exit behavior, removing the temporary probe file while preserving node exit code, and selecting the intended structured provider result from noisy output.
    • Evidence: The test asserts the positive path (`parsed` exists, `probe.exitCode` is `0`, `status` is `available`, and `baseUrl` is `https://inference.local/v1\`\), while `parseProbeJson()` simply picks the first line starting with `{"providerKey"` and no negative-path tests cover shell cleanup or parsing drift.

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 linked an issue Jun 19, 2026 that may be closed by this pull request
79 tasks
@cv
cv merged commit c7962dd into main Jun 19, 2026
44 checks passed
@cv
cv deleted the e2e-migrate/test-cron-preflight-inference-local branch June 19, 2026 21:13
@wscurran wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure refactor PR restructures code without intended behavior change labels Aug 1, 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 refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants