test(e2e): migrate test-model-router-provider-routed-inference.sh to vitest [IND-2]#5221
Merged
jyaunches merged 7 commits intoJun 12, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Vitest live end-to-end test for provider-routed model-router inference, updates workflow selector/matrix/validator logic to register the new scenario/job, adds a gated CI job that optionally logs into Docker Hub (with retries), runs the targeted Vitest test, and includes the job in PR result reporting. ChangesProvider-routed inference end-to-end scenario
Sequence Diagram(s)sequenceDiagram
participant Test
participant CLI as "nemoclaw CLI"
participant Sandbox
participant HealthAPI as "127.0.0.1:4000/health"
participant InferenceAPI as "sandbox /v1/chat/completions"
Test->>CLI: verify CLI exists
Test->>Sandbox: destroy existing sandbox
Test->>CLI: nemoclaw onboard --fresh (NEMOCLAW_PROVIDER=routed)
Test->>HealthAPI: poll /health (up to 20)
HealthAPI->>Test: healthy_count >= 1
Test->>InferenceAPI: POST completion (up to 3 attempts)
InferenceAPI->>Test: completion JSON (model, choices)
Test->>Test: assert model prefix and PONG in choices
Test->>Test: write scenario-result.json
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/e2e-vitest-scenarios.yaml (1)
266-290: 💤 Low valueOptional: Consider extracting Docker Hub authentication if pattern grows.
The Docker Hub authentication block (lines 266-290) is identical to the one in
openclaw-tui-chat-correlation-vitest(lines 344-368). While GitHub Actions lacks native step-reuse between jobs, if more e2e jobs adopt this authentication pattern, consider extracting it to a composite action under.github/actions/to reduce maintenance burden.For now, with only two call sites, the duplication is acceptable.
[optional_refactor]
🤖 Prompt for 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. In @.github/workflows/e2e-vitest-scenarios.yaml around lines 266 - 290, The repeated "Authenticate to Docker Hub" step that uses DOCKERHUB_USERNAME and DOCKERHUB_TOKEN is duplicated across workflows; extract this shell block (the for-loop login with timeout and retry logic) into a reusable composite GitHub Action under .github/actions/docker-hub-auth (or similar) and replace the inline step in the workflows with a single call to that composite action, passing secrets.DOCKERHUB_USERNAME and secrets.DOCKERHUB_TOKEN as inputs so both jobs (e.g., the step named "Authenticate to Docker Hub" in this workflow and the identical step in openclaw-tui-chat-correlation-vitest) reuse the same implementation and reduce duplication.
🤖 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.
Nitpick comments:
In @.github/workflows/e2e-vitest-scenarios.yaml:
- Around line 266-290: The repeated "Authenticate to Docker Hub" step that uses
DOCKERHUB_USERNAME and DOCKERHUB_TOKEN is duplicated across workflows; extract
this shell block (the for-loop login with timeout and retry logic) into a
reusable composite GitHub Action under .github/actions/docker-hub-auth (or
similar) and replace the inline step in the workflows with a single call to that
composite action, passing secrets.DOCKERHUB_USERNAME and secrets.DOCKERHUB_TOKEN
as inputs so both jobs (e.g., the step named "Authenticate to Docker Hub" in
this workflow and the identical step in openclaw-tui-chat-correlation-vitest)
reuse the same implementation and reduce duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 129bb8ae-1fa4-42f4-b6d2-74b15e8c79c4
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/model-router-provider-routed-inference.test.ts
Contributor
PR Review AdvisorFindings: 0 needs attention, 5 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/e2e-vitest-scenarios.yaml (2)
50-64: 💤 Low valueJOBS env variable set but not passed to matrix generator.
The
JOBSenvironment variable is set at line 50 and validated at lines 61-64, but is not passed to the matrix generation script at line 65. This is intentional—theinputs.jobsfilter controls which workflow jobs execute (via job-levelifconditions), not which scenarios appear in the matrix. However, this may not be immediately obvious to maintainers.📝 Optional: Add clarifying comment
- id: matrix name: Generate Vitest scenario matrix env: SCENARIOS: ${{ inputs.scenarios }} + # JOBS is validated here but not passed to the matrix generator; + # it controls job execution via workflow-level if conditions. JOBS: ${{ inputs.jobs }}🤖 Prompt for 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. In @.github/workflows/e2e-vitest-scenarios.yaml around lines 50 - 64, Add a short clarifying comment above the JOBS validation block explaining that JOBS (from inputs.jobs) is validated to control which workflow jobs run via job-level if conditions and is intentionally not added to the matrix generation args (args+=(--scenarios ...)), so maintainers understand that matrix generation vs job filtering are separate concerns; reference the JOBS env variable, inputs.jobs, the args array, and the matrix generation invocation in the comment so it's clear why JOBS isn’t appended to args.
279-304: ⚖️ Poor tradeoffDocker Hub authentication logic duplicated across jobs.
The retry logic at lines 279-304 (in
model-router-provider-routed-inference-vitest) and lines 357-382 (inopenclaw-tui-chat-correlation-vitest) is identical. While the logic is correct, this duplication could be eliminated by extracting it into a reusable composite action.This can be deferred to a future refactor focused on workflow helpers.
Also applies to: 357-382
🤖 Prompt for 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. In @.github/workflows/e2e-vitest-scenarios.yaml around lines 279 - 304, Duplicate "Authenticate to Docker Hub" step appears in the model-router-provider-routed-inference-vitest and openclaw-tui-chat-correlation-vitest jobs; extract this retry/login logic into a reusable composite GitHub Action (e.g., actions/docker-hub-login-composite) that accepts inputs DOCKERHUB_USERNAME and DOCKERHUB_TOKEN and implements the same set -euo pipefail + retry loop + timeout behavior, then replace the inline step in both jobs with a single uses: ./path/to/composite action invocation passing the secrets as inputs to remove duplication.
🤖 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.
Nitpick comments:
In @.github/workflows/e2e-vitest-scenarios.yaml:
- Around line 50-64: Add a short clarifying comment above the JOBS validation
block explaining that JOBS (from inputs.jobs) is validated to control which
workflow jobs run via job-level if conditions and is intentionally not added to
the matrix generation args (args+=(--scenarios ...)), so maintainers understand
that matrix generation vs job filtering are separate concerns; reference the
JOBS env variable, inputs.jobs, the args array, and the matrix generation
invocation in the comment so it's clear why JOBS isn’t appended to args.
- Around line 279-304: Duplicate "Authenticate to Docker Hub" step appears in
the model-router-provider-routed-inference-vitest and
openclaw-tui-chat-correlation-vitest jobs; extract this retry/login logic into a
reusable composite GitHub Action (e.g., actions/docker-hub-login-composite) that
accepts inputs DOCKERHUB_USERNAME and DOCKERHUB_TOKEN and implements the same
set -euo pipefail + retry loop + timeout behavior, then replace the inline step
in both jobs with a single uses: ./path/to/composite action invocation passing
the secrets as inputs to remove duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ed446ae5-7576-43dd-948c-687999e63ef3
📒 Files selected for processing (1)
.github/workflows/e2e-vitest-scenarios.yaml
…l-router-provider-routed-simple # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
Contributor
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27377303092
|
cv
approved these changes
Jun 11, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml:
- Line 1125: Update the job's if condition so it also checks inputs.scenarios
(like the working inference-routing-vitest job) to handle
free_standing_scenarios; specifically, wherever the current condition uses
(inputs.jobs == '' && inputs.scenarios == '') || contains(format(',{0},',
inputs.jobs), ',model-router-provider-routed-inference-vitest,'), mirror that
logic to also test contains(format(',{0},', inputs.scenarios),
',model-router-provider-routed-inference,') (or the exact scenario name used in
free_standing_scenarios) so the job runs when selected via inputs.scenarios as
well as inputs.jobs.
- Line 43: The workflow dispatch/validator is missing the new scenario IDs:
update tools/e2e-scenarios/workflow-boundary.mts to include both
"model-router-provider-routed-inference-vitest" and
"model-router-provider-routed-inference" in the allowed/validated job/scenario
lists, and modify .github/workflows/e2e-vitest-scenarios.yaml so the job
"model-router-provider-routed-inference-vitest" checks both inputs.jobs and
inputs.scenarios (not only inputs.jobs) — specifically adjust the job's if
condition that currently gates on inputs.jobs and ensure free_standing_scenarios
includes the matching id so selecting via inputs.scenarios will dispatch the
job.
🪄 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: 465cc85d-c2fd-438b-b0b9-068c7ca6f987
📒 Files selected for processing (1)
.github/workflows/e2e-vitest-scenarios.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml:
- Line 1129: The job-level env sets DOCKER_CONFIG to ${{ runner.temp
}}/docker-config-model-router-provider-routed-inference but the runner context
isn't available at job-level; move the DOCKER_CONFIG setting out of the
jobs.<job_id>.env and instead add an env DOCKER_CONFIG with the same value into
each step that needs it (for example the "Authenticate to Docker Hub" and "Clean
up Docker auth" steps), or replace the job-level value with a job-safe path such
as ${{ github.workspace }}/.docker-config-model-router-provider-routed-inference
if you must keep it at job-level; update references to DOCKER_CONFIG
accordingly.
In `@tools/e2e-scenarios/workflow-boundary.mts`:
- Around line 1299-1307: The validator currently checks job-level env via
jobEnv.DOCKER_CONFIG against the runner.temp pattern (the block reading const
jobEnv = asRecord(job.env) and the DOCKER_CONFIG comparison); update this to
stop asserting a job-level runner.temp usage — either remove this job-level
DOCKER_CONFIG check or change it to inspect step-level envs (e.g., iterate
job.steps and check each step.env for DOCKER_CONFIG) and add corresponding
assertions in the Docker login and cleanup step validators (the checks around
the Docker auth and cleanup step validations) to ensure DOCKER_CONFIG is present
on those specific steps instead of at the job level.
🪄 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: 23b8d20f-4255-4c67-8f1e-9d6382e5dfa4
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltools/e2e-scenarios/workflow-boundary.mts
| runs-on: ubuntu-latest | ||
| timeout-minutes: 45 | ||
| env: | ||
| DOCKER_CONFIG: ${{ runner.temp }}/docker-config-model-router-provider-routed-inference |
Contributor
There was a problem hiding this comment.
runner context is not available at job-level env; workflow will fail.
The runner context (including runner.temp) is only available within step execution, not in the jobs.<job_id>.env block. Per GitHub Actions context availability rules, job-level env only supports: github, needs, strategy, matrix, secrets, inputs, vars.
This will cause the workflow to error when the job starts.
🐛 Proposed fix: move DOCKER_CONFIG to step-level env
Remove DOCKER_CONFIG from job-level env and set it in each step that needs it:
env:
- DOCKER_CONFIG: ${{ runner.temp }}/docker-config-model-router-provider-routed-inference
E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts/vitest/model-router-provider-routed-inferenceThen add to the "Authenticate to Docker Hub" step and "Clean up Docker auth" step:
- name: Authenticate to Docker Hub
env:
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
+ DOCKER_CONFIG: ${{ runner.temp }}/docker-config-model-router-provider-routed-inference
shell: bash
run: | - name: Clean up Docker auth
if: always()
+ env:
+ DOCKER_CONFIG: ${{ runner.temp }}/docker-config-model-router-provider-routed-inference
run: |Alternatively, you can use a path that doesn't require runner.temp, such as ${{ github.workspace }}/.docker-config-model-router-provider-routed-inference, which is available at job-level.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 1129-1129: context "runner" is not allowed here. available contexts are "github", "inputs", "matrix", "needs", "secrets", "strategy", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🤖 Prompt for 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.
In @.github/workflows/e2e-vitest-scenarios.yaml at line 1129, The job-level env
sets DOCKER_CONFIG to ${{ runner.temp
}}/docker-config-model-router-provider-routed-inference but the runner context
isn't available at job-level; move the DOCKER_CONFIG setting out of the
jobs.<job_id>.env and instead add an env DOCKER_CONFIG with the same value into
each step that needs it (for example the "Authenticate to Docker Hub" and "Clean
up Docker auth" steps), or replace the job-level value with a job-safe path such
as ${{ github.workspace }}/.docker-config-model-router-provider-routed-inference
if you must keep it at job-level; update references to DOCKER_CONFIG
accordingly.
Source: Linters/SAST tools
Comment on lines
+1299
to
+1307
| const jobEnv = asRecord(job.env); | ||
| if ( | ||
| jobEnv.DOCKER_CONFIG !== | ||
| "${{ runner.temp }}/docker-config-model-router-provider-routed-inference" | ||
| ) { | ||
| errors.push( | ||
| "model-router-provider-routed-inference-vitest job must isolate Docker auth with DOCKER_CONFIG under runner.temp", | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Validator expects invalid job-level runner.temp usage.
This validation enforces the exact pattern that actionlint flagged as invalid in the workflow (runner context unavailable at job-level env). When the workflow is fixed by moving DOCKER_CONFIG to step-level env, this validator block should be updated accordingly.
♻️ Suggested adjustment after workflow fix
If the workflow fix moves DOCKER_CONFIG to step-level env (in Docker auth and cleanup steps), update this validator to either remove the job-level check or validate step-level env instead:
- if (
- jobEnv.DOCKER_CONFIG !==
- "${{ runner.temp }}/docker-config-model-router-provider-routed-inference"
- ) {
- errors.push(
- "model-router-provider-routed-inference-vitest job must isolate Docker auth with DOCKER_CONFIG under runner.temp",
- );
- }
+ // DOCKER_CONFIG is set at step-level (Docker auth and cleanup steps) where runner context is availableThen add validation in the Docker login and cleanup step checks (around lines 1367-1383 and 1456-1461) to verify DOCKER_CONFIG is set in those step-level envs.
🤖 Prompt for 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.
In `@tools/e2e-scenarios/workflow-boundary.mts` around lines 1299 - 1307, The
validator currently checks job-level env via jobEnv.DOCKER_CONFIG against the
runner.temp pattern (the block reading const jobEnv = asRecord(job.env) and the
DOCKER_CONFIG comparison); update this to stop asserting a job-level runner.temp
usage — either remove this job-level DOCKER_CONFIG check or change it to inspect
step-level envs (e.g., iterate job.steps and check each step.env for
DOCKER_CONFIG) and add corresponding assertions in the Docker login and cleanup
step validators (the checks around the Docker auth and cleanup step validations)
to ensure DOCKER_CONFIG is present on those specific steps instead of at the job
level.
This was referenced Jun 12, 2026
This was referenced Jun 12, 2026
79 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate
test/e2e/test-model-router-provider-routed-inference.shwith focused live Vitest coverage.Related Issues
Refs #5098
Refs #3255
Contract mapping
test/e2e-scenario/live/model-router-provider-routed-inference.test.tschecksdocker info.NVIDIA_API_KEYis present andnvapi--prefixed.nemoclaw onboard --freshsucceeds withNEMOCLAW_PROVIDER=routed,NEMOCLAW_PROVIDER_KEY, and open policy tier.node bin/nemoclaw.js onboard --fresh --non-interactive --yes-i-accept-third-party-softwarewith the same routed env.nemoclaw <sandbox> destroy.curl http://127.0.0.1:4000/healthand assertinghealthy_count > 0.inference.localreturns a routed Model Router PONG completion.openshell sandbox exec ... curl https://inference.local/v1/chat/completionsassertsmodelstarts withnvidia-routedand content includesPONG.Simplicity check
e2e-vitest-scenarios.yamljob for this free-standing live test; legacy bash script andregression-e2e.yamllane deletion are deferred to Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 Phase 11.Verification
NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/model-router-provider-routed-inference.test.ts --typecheck.enabled false(local skip: missing live prerequisites/secrets)npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts --typecheck.enabled falsenpm run build:clinpm run typecheck:clinpx @biomejs/biome lint test/e2e-scenario/live/model-router-provider-routed-inference.test.tsgit diff --checkSummary by CodeRabbit
Tests
Chores