test(e2e): retire onboard inference smoke script

[jyaunches]

Summary

Retires the legacy caller-level onboard inference smoke bash regression now that the same contract is covered by a focused Vitest process test.

Related Issues

Refs #3253
Refs #4349
Refs #5098
Refs #5119

Changes

• Adds test/onboard-inference-smoke.test.ts, which drives setupInference() in a subprocess with PATH-shimmed openshell and curl.
• Verifies onboard probes /chat/completions, fails closed on upstream HTTP 503, surfaces provider/model/API base/credential diagnostics, and does not print route success after smoke failure.
• Deletes test/e2e/test-onboard-inference-smoke.sh.
• Removes the retired onboard-inference-smoke-e2e lane from regression-e2e.yaml.
• Updates the post-test(e2e): simplify legacy migration tracking #5126 frozen legacy E2E shell allowlist and regression workflow contract test to keep the retired lane out of dispatch selection.

Simplicity check

• Test shape: focused process Vitest test.
• New shared helpers: none.
• New framework/registry/ledger: none.
• Workflow changes: remove retired regression lane and update frozen shell/workflow contract allowlists.

Verification

• npm run build:cli
• npx vitest run test/onboard-inference-smoke.test.ts
• npx tsc --noEmit -p jsconfig.json
• npx vitest run --project cli test/onboard-inference-smoke.test.ts test/regression-e2e-workflow.test.ts test/e2e-script-workflow.test.ts --silent=false --reporter=default
• git diff --check

Notes:

• A local full hook/all-files run was attempted and failed on existing environment-dependent failures unrelated to this change (missing nested nemoclaw/dist / nemoclaw/node_modules/json5, Docker daemon unavailable, and several local timeout-sensitive tests). CI should run the repository's normal validation in the correct environment.

Type of Change

• Test-only change
• CI/workflow cleanup for retired legacy coverage
• No secrets, API keys, or credentials committed

Summary by CodeRabbit

• Tests

  • Migrated onboard inference smoke test from shell script to Vitest-based test for improved reliability and isolation.

• Chores

  • Removed onboard-inference-smoke-e2e from regression workflow selectable jobs.

[coderabbitai]

Warning

Review limit reached

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

More reviews will be available in 9 minutes and 26 seconds. 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: a5cdb2f0-191c-404c-a34f-64ab1ca8b2d1

📥 Commits

Reviewing files that changed from the base of the PR and between 744517b and 1a7ba31.

📒 Files selected for processing (3)

• .github/workflows/regression-e2e.yaml
• test/e2e-script-workflow.test.ts
• test/regression-e2e-workflow.test.ts

📝 Walkthrough

Walkthrough

This PR migrates the onboard inference smoke test from a bash E2E script to a Vitest unit test, removes the corresponding job from the regression workflow, and updates contract tests to enforce the migration by preventing the retired job from reappearing.

Changes

Onboard Inference Smoke Test Migration

Layer / File(s)	Summary
New Vitest smoke test implementation test/onboard-inference-smoke.test.ts	A Vitest suite spawns an isolated Node script with mocked curl and overridden runner/registry modules. It invokes setupInference() against an OpenAI-compatible route whose upstream returns HTTP 503, asserts setup rejects the failed probe, verifies diagnostics contain the 503 error and credential configuration, confirms the /chat/completions endpoint was probed, and ensures the route-success message is withheld until the smoke probe succeeds.
Regression workflow cleanup .github/workflows/regression-e2e.yaml	Removes the onboard-inference-smoke-e2e job definition, its selector output (onboard_inference_smoke), the workflow dispatch job name from the selectable input list, and the shell conditional logic that set the selector output based on user input.
Test contract updates test/regression-e2e-workflow.test.ts, test/e2e-script-workflow.test.ts	Refactors the regression E2E workflow contract test to use parameterized it.each covering retired lanes, asserting retired lane names and selector outputs do not appear in the workflow dispatch description, job keys, or selector logic. Removes the legacy bash script from the E2E allowlist expectation.

---

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

---

Possibly related PRs

• NVIDIA/NemoClaw#5119: Modifies .github/workflows/regression-e2e.yaml and test/regression-e2e-workflow.test.ts to retire a regression lane and update contract coverage assertions.

---

Suggested reviewers

• cv
• prekshivyas

---

Poem

🐰 A bash script hops to Vitest's test floor,
Where mocked curl probes the 503 door,
Old workflows retire with grace and care,
New contracts ensure none slip back in there!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: retiring the legacy onboard inference smoke bash script and replacing it with focused Vitest coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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-retire/test-onboard-inference-smoke

---

_{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 is recommended for this PR because the changes are limited to CI workflow/test coverage migration and deletion of a retired regression lane. They do not modify NemoClaw runtime, installer/onboarding implementation, sandbox lifecycle, credentials, network policy, inference routing code, deployment logic, or real assistant user flows. The relevant validation should be the normal unit/Vitest workflow-contract tests rather than dispatching runtime E2E jobs.

Optional E2E

• None.

New E2E recommendations

• onboarding-inference-coverage (low): If maintainers still want dispatchable end-to-end coverage for issue [Brev][Onboard] Onboard reports SUCCESS without exercising inference path — config bugs reach end users instead of being caught #3253 after retiring onboard-inference-smoke-e2e, add a scenario-level E2E that exercises onboarding through the public CLI against a runtime-broken OpenAI-compatible inference route and verifies the CLI fails before printing installation success. Do not re-add the deleted direct setupInference bash probe unless that is the intended coverage surface.
  • Suggested test: Scenario E2E for onboarding failure on runtime-broken inference route

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• None. Changes are limited to the regression E2E workflow, legacy bash E2E retirement, and non-scenario Vitest tests outside test/e2e-scenario/. They do not affect the Vitest scenario workflow, scenario registry/runtime support, live scenario entry point, or shared scenario fixtures.

Optional Vitest E2E scenarios

• None.

Relevant changed files

• None.

[github-actions]

PR Review Advisor

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

Consider writing more tests for

• **Runtime validation** — Subprocess timeout or signal termination in `test/onboard-inference-smoke.test.ts` fails with a timeout/signal-specific diagnostic instead of being mistaken for the expected onboard smoke rejection.. The core negative-path behavior is covered by the new Vitest subprocess test, and the regression workflow contract test covers removal from dispatch selection. Because this PR removes a manually dispatched workflow lane and artifact path, a small amount of additional runtime-boundary coverage would further harden confidence but is not required for the current diff.
• **Runtime validation** — Successful compatible-endpoint smoke updates `registry.updateSandbox` and prints `Inference route set` only after `/chat/completions` returns 2xx.. The core negative-path behavior is covered by the new Vitest subprocess test, and the regression workflow contract test covers removal from dispatch selection. Because this PR removes a manually dispatched workflow lane and artifact path, a small amount of additional runtime-boundary coverage would further harden confidence but is not required for the current diff.

Workflow run details

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

[wscurran]

✨
Related open PRs:

• #5119 test(e2e): retire docker-unreachable gateway script

---

Related open issues:

• #5098 [DRAFT] Epic: Migrate test/e2e bash suite to typed Vitest E2E scenarios
• #4349 Phase 3: Inference Provider, Routing, and Config-Shape Audit Coverage (E2E audit-coverage)
• #3253 [Brev][Onboard] Onboard reports SUCCESS without exercising inference path — config bugs reach end users instead of being caught

[cv]

Coordination note now that #5126 is green: this PR is still based on the pre-#5126 migration shape.

On the intended base, test/e2e-scenario/migration/legacy-inventory.json no longer exists and should not be reintroduced. We also should not use a special Legacy E2E deletion evidence PR-body block. The replacement shape for deleting a legacy script is deterministic instead:

• port the behavioral coverage into Vitest
• delete the legacy test/e2e/test-*.sh entry point only when replacement coverage/wiring lands in the same PR
• update the frozen top-level script allowlist in test/e2e-script-workflow.test.ts
• if the script is wired in regression-e2e.yaml, remove/replace that lane and update test/regression-e2e-workflow.test.ts

This appears to overlap #5149, which already carries the onboard-inference-smoke slice on the #5126-based draft stack. My recommendation is to close this as superseded by #5149 unless it contains a distinct assertion that should be ported there.

[jyaunches]

Migration-principle review for #5098: the core direction here is aligned — focused Vitest coverage is the right replacement for this caller-level onboard inference smoke probe.

Reference from test/e2e-scenario/docs/MIGRATION.md: Vitest is the harness, fixtures are support code, and GitHub PRs/issues are migration source of truth. For caller-level smoke probes, keep the replacement in focused test/ coverage rather than creating live E2E fixture/registry work.

Recommended path:

• Keep this as a small focused retirement: test/onboard-inference-smoke.test.ts + delete test/e2e/test-onboard-inference-smoke.sh + remove the regression lane + update the existing workflow contract test.
• Do not add live E2E fixture, registry scenario, or migration framework work for this script.

Cleanup requested before merge/rebase:

• Drop legacy-inventory.json changes after test(e2e): simplify legacy migration tracking #5126; GitHub PRs/issues are the source of truth now.
• Avoid “modern Vitest framework” / framework wording; call it focused Vitest process coverage.
• Add a compact Simplicity check section:
  • Test shape: focused process Vitest test.
  • New shared helpers: none.
  • New framework/registry/ledger: none.
  • Workflow changes: remove retired regression lane.

[prekshivyas]

Reviewed (code + 9-cat security). Retires test/e2e/test-onboard-inference-smoke.sh + its dispatch lane, replacing it with a Vitest subprocess test that drives setupInference() with a PATH-shimmed openshell/curl.

✅ Approve — coverage preserved and strengthened: the new test sits at top-level test/ (inside the cli project's include, outside the test/e2e/** exclude), so a dispatch-only lane becomes always-on per-PR coverage. Verified end-to-end: production verifyOnboardInferenceSmoke → runCurlProbe → spawnSync("curl") emits every diagnostic the test asserts and process.exit(1) guarantees the route-success line is suppressed; the test correctly sets VITEST="false" so the real smoke runs (not the VITEST==="true" short-circuit). Inventory/contract edits are consistent with no stale refs.

Security: all 9 pass — fake key only, loopback/unroutable endpoint, redacted diagnostics. Nit: the runner.run/runCapture stubs are slightly more mocking than needed (the assertion-bearing probe goes via spawnSync, not the runner).