ci(e2e): route shared jobs to NVIDIA endpoints - #5698
Conversation
|
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:
📝 WalkthroughWalkthroughThe reusable E2E script runner gains a unified "Export CI inference environment" step that branches on ChangesNVIDIA Inference Credential Routing for E2E CI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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-script-workflow.test.ts`:
- Around line 914-936: The workflowCall variable is missing the secrets property
in its type definition, causing a type error when asserting on
workflowCall?.secrets?.NVIDIA_API_KEY. Update the type definition for the
workflow_call contract (from either runnerWorkflow.on?.workflow_call or
runnerWorkflow.true?.workflow_call) to include a secrets property with the
appropriate structure, or cast workflowCall to a type that includes the secrets
property so the assertion on line 936 will compile.
🪄 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: 16903b86-ef60-4d98-836f-726adfa2839d
📒 Files selected for processing (5)
.github/workflows/e2e-script.yaml.github/workflows/nightly-e2e.yamltest/e2e-scenario/support-tests/hosted-inference.test.tstest/e2e-script-workflow.test.tstest/e2e/lib/ci-compatible-inference.sh
|
Updated for the review feedback:
|
There was a problem hiding this comment.
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-script-workflow.test.ts`:
- Around line 445-453: The `nvidiaBuildJobs` set is missing
`messaging-providers-e2e` even though this job is treated as an NVIDIA-build job
for secrets handling at line 468 with `nvidiaBuildSecrets`. This inconsistency
causes the routing assertions (around lines 1037-1043) to skip validation of
`inference_route` for that job, creating incomplete test coverage. Add
`messaging-providers-e2e` to the `nvidiaBuildJobs` set to align its
classification across both the secrets and routing validation logic.
🪄 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: 144fdc78-0fc9-49b9-bf44-0656867afc64
📒 Files selected for processing (3)
.github/workflows/e2e-script.yaml.github/workflows/nightly-e2e.yamltest/e2e-script-workflow.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/lib/ci-compatible-inference.sh (1)
31-47: 🗄️ Data Integrity & Integration | 🟠 MajorClarify intent of two different endpoint domains across compatible and non-compatible inference modes.
Line 42 hardcodes the endpoint to
https://inference-api.nvidia.com/v1for compatible inference mode, while the new constantNEMOCLAW_E2E_NVIDIA_INFERENCE_ENDPOINT_DEFAULT(line 16) useshttps://integrate.api.nvidia.com/v1for non-compatible mode. The functionnemoclaw_e2e_hosted_inference_base_url()(lines 53–59) reflects this split: it returns the compatible-mode endpoint when compatible inference is enabled (line 55) and the new constant otherwise (line 57).Confirm whether:
- Using different endpoints for compatible vs. non-compatible modes is intentional
- If both should use the same endpoint, line 42 should reference the constant for consistency
🤖 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 `@test/e2e/lib/ci-compatible-inference.sh` around lines 31 - 47, Review the endpoint URL configuration in the nemoclaw_e2e_configure_compatible_inference function where NEMOCLAW_ENDPOINT_URL is hardcoded to https://inference-api.nvidia.com/v1 on line 42, and compare it against the NEMOCLAW_E2E_NVIDIA_INFERENCE_ENDPOINT_DEFAULT constant defined at line 16 which uses https://integrate.api.nvidia.com/v1. Determine whether these endpoints should be the same or intentionally different. If they should be the same, replace the hardcoded URL on line 42 with a reference to the constant for consistency. If they are intentionally different, add a clear comment above line 42 explaining why the compatible inference mode uses a different endpoint from the non-compatible mode.
🤖 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.
Outside diff comments:
In `@test/e2e/lib/ci-compatible-inference.sh`:
- Around line 31-47: Review the endpoint URL configuration in the
nemoclaw_e2e_configure_compatible_inference function where NEMOCLAW_ENDPOINT_URL
is hardcoded to https://inference-api.nvidia.com/v1 on line 42, and compare it
against the NEMOCLAW_E2E_NVIDIA_INFERENCE_ENDPOINT_DEFAULT constant defined at
line 16 which uses https://integrate.api.nvidia.com/v1. Determine whether these
endpoints should be the same or intentionally different. If they should be the
same, replace the hardcoded URL on line 42 with a reference to the constant for
consistency. If they are intentionally different, add a clear comment above line
42 explaining why the compatible inference mode uses a different endpoint from
the non-compatible mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a931737-f096-49a2-a31f-15558cd621b8
📒 Files selected for processing (3)
.github/workflows/e2e-script.yamltest/e2e-script-workflow.test.tstest/e2e/lib/ci-compatible-inference.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/e2e-script.yaml
- test/e2e-script-workflow.test.ts
Selective E2E Results — ❌ Some jobs failedRun: 28065470733
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Selective E2E Results — ❌ Some jobs failedRun: 28066992295
|
Selective E2E Results — ✅ All requested jobs passedRun: 28068414918
|
wscurran
added
area: ci
There was a problem hiding this comment.
Hey @jyaunches, this looks clean: advisor merge_as_is, CodeRabbit clear, latest selective E2E passed. Can you merge main to clear the dirty state, then re-trigger the nightly E2E? Once that's green we can merge
## Summary - Add an explicit reusable workflow input, `inference_route`, for `e2e-script.yaml`. - Keep the default route unchanged: `hosted-custom`. - Convert selected Group B timeout jobs to `inference_route: nvidia-internal` as an auditable opt-in label. - Keep `NVIDIA_API_KEY` out of the reusable workflow contract; both hosted labels use `NVIDIA_INFERENCE_API_KEY` and `COMPATIBLE_API_KEY` for the hosted-compatible internal endpoint. ## Converted Group B jobs - `messaging-providers-e2e` - `channels-add-remove-e2e` - `channels-stop-start-openclaw-e2e` - `channels-stop-start-hermes-e2e` - `hermes-discord-e2e` - `upgrade-stale-sandbox-e2e` - `rebuild-hermes-e2e` - `rebuild-hermes-stale-base-e2e` ## Intentionally not converted - `common-egress-agent-e2e` stays on the existing default route for now because its original post-revert failure was `exit 124` / empty agent reply, not the hosted setup timeout or public-endpoint quota signature. ## Routing behavior - `hosted-custom` remains the default legacy route. - `nvidia-internal` is intentionally equivalent to `hosted-custom` at runtime, but lets changed jobs opt in explicitly so reviewers can see which jobs moved away from the rejected public NVIDIA Endpoints/Build route. - Both labels export: - `NVIDIA_INFERENCE_API_KEY` from `secrets.NVIDIA_INFERENCE_API_KEY` - `NEMOCLAW_E2E_USE_HOSTED_INFERENCE=1` - `NEMOCLAW_PROVIDER=custom` - `NEMOCLAW_ENDPOINT_URL=https://inference-api.nvidia.com/v1` - `NEMOCLAW_MODEL=nvidia/nvidia/nemotron-3-super-v3` - `NEMOCLAW_COMPAT_MODEL=nvidia/nvidia/nemotron-3-super-v3` - `NEMOCLAW_PREFERRED_API=openai-completions` - `COMPATIBLE_API_KEY` - Unsupported `inference_route` values fail closed without echoing raw caller input. - Hosted inference secrets are rejected before `GITHUB_ENV` export if they contain CR/LF. ## Validation - `npx biome check --write .github/workflows/e2e-script.yaml test/e2e-script-workflow.test.ts` - `git diff --check` - `npm test -- test/e2e-script-workflow.test.ts test/e2e-scenario/support-tests/hosted-inference.test.ts` - `npm run typecheck:cli` - `npm run source-shape:check` ## E2E status - Public NVIDIA Endpoints/Build route was rejected after run `28065470733` hit HTTP 429. - Internal route first failed with HTTP 401 on model `nvidia/nemotron-3-super-v3`; `cloud-onboard-e2e` proved the hosted internal service accepts the legacy `nvidia/nvidia/nemotron-3-super-v3` alias. - Latest head still needs required nightly E2E rerun after PR CI/advisors are green. --------- Co-authored-by: Carlos Villela <cvillela@nvidia.com>
5 participants
Summary
inference_route, fore2e-script.yaml.hosted-custom.inference_route: nvidia-internalas an auditable opt-in label.NVIDIA_API_KEYout of the reusable workflow contract; both hosted labels useNVIDIA_INFERENCE_API_KEYandCOMPATIBLE_API_KEYfor the hosted-compatible internal endpoint.Converted Group B jobs
messaging-providers-e2echannels-add-remove-e2echannels-stop-start-openclaw-e2echannels-stop-start-hermes-e2ehermes-discord-e2eupgrade-stale-sandbox-e2erebuild-hermes-e2erebuild-hermes-stale-base-e2eIntentionally not converted
common-egress-agent-e2estays on the existing default route for now because its original post-revert failure wasexit 124/ empty agent reply, not the hosted setup timeout or public-endpoint quota signature.Routing behavior
hosted-customremains the default legacy route.nvidia-internalis intentionally equivalent tohosted-customat runtime, but lets changed jobs opt in explicitly so reviewers can see which jobs moved away from the rejected public NVIDIA Endpoints/Build route.NVIDIA_INFERENCE_API_KEYfromsecrets.NVIDIA_INFERENCE_API_KEYNEMOCLAW_E2E_USE_HOSTED_INFERENCE=1NEMOCLAW_PROVIDER=customNEMOCLAW_ENDPOINT_URL=https://inference-api.nvidia.com/v1NEMOCLAW_MODEL=nvidia/nvidia/nemotron-3-super-v3NEMOCLAW_COMPAT_MODEL=nvidia/nvidia/nemotron-3-super-v3NEMOCLAW_PREFERRED_API=openai-completionsCOMPATIBLE_API_KEYinference_routevalues fail closed without echoing raw caller input.GITHUB_ENVexport if they contain CR/LF.Validation
npx biome check --write .github/workflows/e2e-script.yaml test/e2e-script-workflow.test.tsgit diff --checknpm test -- test/e2e-script-workflow.test.ts test/e2e-scenario/support-tests/hosted-inference.test.tsnpm run typecheck:clinpm run source-shape:checkE2E status
28065470733hit HTTP 429.nvidia/nemotron-3-super-v3;cloud-onboard-e2eproved the hosted internal service accepts the legacynvidia/nvidia/nemotron-3-super-v3alias.