fix(e2e): route nightly hosted inference as custom#5385
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRoutes CI and E2E inference to a hosted OpenAI-compatible endpoint: add hosted credential staging in onboarding, switch CI helper to hosted mode, update workflows to export guarded hosted env, modify many E2E scripts to source/configure the helper, and update tests to assert hosted env wiring. ChangesHosted Inference CI Infrastructure Migration
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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 |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 44%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: 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
|
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/test-launchable-smoke.sh (1)
489-500:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRetry attempt uses a stale hardcoded model
Line 499 ignores the configured hosted model and reverts to a fixed model id. If the first attempt fails transiently, retries may hit the wrong model and fail for non-product reasons.
Suggested fix
- -d '{\"model\":\"nvidia/nemotron-3-super-120b-a12b\",\"messages\":[{\"role\":\"user\",\"content\":\"Reply with exactly one word: PONG\"}],\"max_tokens\":100}'" \ + -d '{\"model\":\"$HOSTED_INFERENCE_MODEL\",\"messages\":[{\"role\":\"user\",\"content\":\"Reply with exactly one word: PONG\"}],\"max_tokens\":100}'" \🤖 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/test-launchable-smoke.sh` around lines 489 - 500, The retry block in the run_with_timeout call builds a curl -d JSON payload that hardcodes "model":"nvidia/nemotron-3-super-120b-a12b" (inside the ssh/curl invocation); replace that literal with the configured hosted-model variable used elsewhere in the script (e.g. $HOSTED_MODEL or the script's model variable) so the retry uses the same model as the primary attempt, preserving proper JSON quoting/escaping when interpolating the shell variable into the -d argument; update the curl payload in the run_with_timeout/ssh command to use the variable and ensure quoting is safe for SSH/JSON.
🧹 Nitpick comments (1)
test/e2e-script-workflow.test.ts (1)
904-929: ⚡ Quick winBroaden the direct-job hosted-inference contract table.
This only asserts five direct lanes, but the migration in
.github/workflows/nightly-e2e.yamlalso rewired other direct hosted-secret jobs likeopenclaw-tui-chat-correlation-e2e,sandbox-operations-e2e,onboard-repair-e2e,onboard-resume-e2e,onboard-negative-paths-e2e, andruntime-overrides-e2e. A drift in any of those env blocks would still leave this test green. Please either derive the cases from the workflow or enumerate the full direct-job set touched by this contract.🤖 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-script-workflow.test.ts` around lines 904 - 929, Test only asserts five direct lanes (directJobSteps) but the workflow rewired additional direct hosted-secret jobs; update the assertion set so the test covers all direct hosted-secret jobs. Fix by either (A) expanding the directJobSteps array in test/e2e-script-workflow.test.ts to include the full list of job names touched by the migration (add openclaw-tui-chat-correlation-e2e, sandbox-operations-e2e, onboard-repair-e2e, onboard-resume-e2e, onboard-negative-paths-e2e, runtime-overrides-e2e in addition to the existing entries) and keep the same stepName pairs, or (B) compute the cases from nightlyWorkflow by deriving job entries from nightlyWorkflow.jobs (e.g., filter Object.entries(nightlyWorkflow.jobs) for jobs whose steps contain the targeted step name and env values like NEMOCLAW_E2E_USE_HOSTED_INFERENCE === "1" and NEMOCLAW_PROVIDER === "custom"), then iterate that derived list instead of the hardcoded directJobSteps; update references to directJobSteps and the loop accordingly so the test will fail if any direct-hosted-secret job drifts.
🤖 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/test-agent-turn-latency-e2e.sh`:
- Around line 585-586: The current invocation
"nemoclaw_e2e_configure_compatible_inference || finish" can hide setup failures
because finish() only causes a non-zero exit when FAIL>0; change the error
handling so a failed konfiguraton causes a non-zero exit: either make finish()
always exit non-zero on immediate failure or replace the invocation with an
explicit failure path like calling finish and then exiting non-zero (for
example: nemoclaw_e2e_configure_compatible_inference || { finish; exit 1; }) so
that a failed nemoclaw_e2e_configure_compatible_inference is reported as a
failure instead of potentially returning success.
In `@test/e2e/test-onboard-negative-paths.sh`:
- Around line 82-91: The script currently derives EXPECTED_PROVIDER from the
hosted-inference helpers but then forces NEMOCLAW_PROVIDER=custom for the env
... node "bin/nemoclaw.js" onboard calls, which breaks non-hosted validation;
update those onboarding invocations to be mode-aware by removing the hardcoded
NEMOCLAW_PROVIDER=custom and instead pass NEMOCLAW_PROVIDER="$EXPECTED_PROVIDER"
(or only set NEMOCLAW_PROVIDER when unset) so the onboard runs follow the
EXPECTED_PROVIDER variable consistently; locate the env ... node "nemoclaw.js"
onboard invocations referenced in the comment and replace the forced custom
provider with the EXPECTED_PROVIDER (or a conditional that preserves existing
behavior if NEMOCLAW_PROVIDER is already set).
- Around line 308-313: Replace the plain non-empty check of RESTORE_API_KEY in
the if block with the mode-aware helper call by invoking the existing helper
function (nemoclaw_e2e_require_hosted_inference_key) instead of checking
RESTORE_API_KEY directly; ensure the helper's return/failure behavior is used
(call it and, on failure, call fail + print_summary + exit 1) and remove the
redundant pass "NVIDIA_INFERENCE_API_KEY is set" or adapt it to run only on
success of the helper so hosted CI remains permissive while local/live runs
still enforce the nvapi- key prefix.
In `@test/e2e/test-openclaw-skill-cli-e2e.sh`:
- Line 77: Call to the helper function
nemoclaw_e2e_configure_compatible_inference should fail the script when it
returns non-zero; update test/e2e/test-openclaw-skill-cli-e2e.sh (lines 77-77)
to append "|| exit 1" after the nemoclaw_e2e_configure_compatible_inference
invocation, and likewise update test/e2e/test-skill-agent-e2e.sh (lines 104-104)
to append "|| exit 1" after its nemoclaw_e2e_configure_compatible_inference call
so the script exits immediately on error.
---
Outside diff comments:
In `@test/e2e/test-launchable-smoke.sh`:
- Around line 489-500: The retry block in the run_with_timeout call builds a
curl -d JSON payload that hardcodes "model":"nvidia/nemotron-3-super-120b-a12b"
(inside the ssh/curl invocation); replace that literal with the configured
hosted-model variable used elsewhere in the script (e.g. $HOSTED_MODEL or the
script's model variable) so the retry uses the same model as the primary
attempt, preserving proper JSON quoting/escaping when interpolating the shell
variable into the -d argument; update the curl payload in the
run_with_timeout/ssh command to use the variable and ensure quoting is safe for
SSH/JSON.
---
Nitpick comments:
In `@test/e2e-script-workflow.test.ts`:
- Around line 904-929: Test only asserts five direct lanes (directJobSteps) but
the workflow rewired additional direct hosted-secret jobs; update the assertion
set so the test covers all direct hosted-secret jobs. Fix by either (A)
expanding the directJobSteps array in test/e2e-script-workflow.test.ts to
include the full list of job names touched by the migration (add
openclaw-tui-chat-correlation-e2e, sandbox-operations-e2e, onboard-repair-e2e,
onboard-resume-e2e, onboard-negative-paths-e2e, runtime-overrides-e2e in
addition to the existing entries) and keep the same stepName pairs, or (B)
compute the cases from nightlyWorkflow by deriving job entries from
nightlyWorkflow.jobs (e.g., filter Object.entries(nightlyWorkflow.jobs) for jobs
whose steps contain the targeted step name and env values like
NEMOCLAW_E2E_USE_HOSTED_INFERENCE === "1" and NEMOCLAW_PROVIDER === "custom"),
then iterate that derived list instead of the hardcoded directJobSteps; update
references to directJobSteps and the loop accordingly so the test will fail if
any direct-hosted-secret job drifts.
🪄 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: 27a452b9-2527-4e87-b24c-5f51eb036195
📒 Files selected for processing (22)
.github/workflows/e2e-script.yaml.github/workflows/nightly-e2e.yamltest/e2e-script-workflow.test.tstest/e2e/lib/ci-compatible-inference.shtest/e2e/test-agent-turn-latency-e2e.shtest/e2e/test-common-egress-agent-e2e.shtest/e2e/test-cron-preflight-inference-local-e2e.shtest/e2e/test-hermes-discord-e2e.shtest/e2e/test-hermes-e2e.shtest/e2e/test-hermes-inference-switch.shtest/e2e/test-hermes-slack-e2e.shtest/e2e/test-issue-4434-tui-unreachable-inference.shtest/e2e/test-launchable-smoke.shtest/e2e/test-onboard-negative-paths.shtest/e2e/test-onboard-repair.shtest/e2e/test-onboard-resume.shtest/e2e/test-openclaw-inference-switch.shtest/e2e/test-openclaw-skill-cli-e2e.shtest/e2e/test-overlayfs-autofix.shtest/e2e/test-sandbox-survival.shtest/e2e/test-shields-config.shtest/e2e/test-skill-agent-e2e.sh
Contributor
PR Review AdvisorFindings: 1 needs attention, 7 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. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/onboard/providers.test.ts (1)
293-324: ⚡ Quick winAdd coverage for the hosted-routing branches nightly CI actually uses.
These tests only lock down the auto-detect path (
unset provider+ non-nvapi-key) and the explicitcloudbypass. The new helper still has separate branches forNEMOCLAW_PROVIDER=customandNEMOCLAW_E2E_USE_HOSTED_INFERENCE=1, and either can regress the hosted CI wiring without this file failing.Suggested additions
+ it("stages hosted inference when the provider is explicitly custom", () => { + withProviderEnv( + { + NVIDIA_INFERENCE_API_KEY: "repo-hosted-key", + NEMOCLAW_PROVIDER: "custom", + }, + () => { + expect(stageHostedInferenceSourceSecretEnv()).toBe(true); + expect(process.env.NEMOCLAW_PROVIDER).toBe("custom"); + expect(process.env.NEMOCLAW_ENDPOINT_URL).toBe(HOSTED_INFERENCE_ENDPOINT_URL); + expect(process.env.COMPATIBLE_API_KEY).toBe("repo-hosted-key"); + }, + ); + }); + + it("stages hosted inference when the hosted flag is enabled for an nvapi key", () => { + withProviderEnv( + { + NVIDIA_INFERENCE_API_KEY: "nvapi-test-key", + NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1", + }, + () => { + expect(stageHostedInferenceSourceSecretEnv()).toBe(true); + expect(getRequestedProviderHint(true)).toBe("custom"); + expect(process.env.COMPATIBLE_API_KEY).toBe("nvapi-test-key"); + }, + ); + });🤖 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 `@src/lib/onboard/providers.test.ts` around lines 293 - 324, The test file currently only covers the auto-detect path and the explicit cloud bypass; add additional test cases to exercise the other hosted-routing branches so CI won't miss regressions: add one test that sets NEMOCLAW_PROVIDER="custom" with a non-nvapi NVIDIA_INFERENCE_API_KEY and asserts stageHostedInferenceSourceSecretEnv() returns true, getRequestedProviderHint(true) returns "custom", and process.env.COMPATIBLE_API_KEY/ NEMOCLAW_* vars are set to the hosted values (mirror the first test but with NEMOCLAW_PROVIDER preset), and add another test that sets NEMOCLAW_E2E_USE_HOSTED_INFERENCE="1" (with/without NVIDIA_INFERENCE_API_KEY as appropriate) and asserts the same hosted-routing outcomes; reference the helper functions stageHostedInferenceSourceSecretEnv, getRequestedProviderHint, getRequestedModelHint and constants HOSTED_INFERENCE_MODEL/HOSTED_INFERENCE_ENDPOINT_URL to implement the assertions exactly as in the existing tests.src/lib/onboard/providers.ts (1)
221-231: ⚡ Quick winDeduplicate provider alias normalization before it drifts.
stageHostedInferenceSourceSecretEnv()now carries a second alias table alongsidegetNonInteractiveProvider(). If one map changes without the other, hosted staging and final provider resolution can disagree for the sameNEMOCLAW_PROVIDERvalue. Extract a shared normalizer and reuse it in both places.Possible cleanup
+function normalizeNonInteractiveProviderKey(providerKey) { + const rawProvider = (providerKey || "").trim().toLowerCase(); + const aliases = { + cloud: "build", + anthropiccompatible: "anthropicCompatible", + hermes: "hermesProvider", + "hermes-provider": "hermesProvider", + hermesprovider: "hermesProvider", + nous: "hermesProvider", + "nous-portal": "hermesProvider", + }; + return aliases[rawProvider] || rawProvider; +} + function getNonInteractiveProvider() { stageHostedInferenceSourceSecretEnv(); - const providerKey = (process.env.NEMOCLAW_PROVIDER || "").trim().toLowerCase(); + const providerKey = normalizeNonInteractiveProviderKey(process.env.NEMOCLAW_PROVIDER); if (!providerKey) return null; - const aliases = { - cloud: "build", - nim: "nim-local", - vllm: "vllm", - anthropiccompatible: "anthropicCompatible", - hermes: "hermesProvider", - "hermes-provider": "hermesProvider", - hermesprovider: "hermesProvider", - nous: "hermesProvider", - "nous-portal": "hermesProvider", - }; - const normalized = aliases[providerKey] || providerKey; + const normalized = providerKey; // ... } function stageHostedInferenceSourceSecretEnv() { const sourceKey = normalizeCredentialValue(process.env[HOSTED_INFERENCE_SOURCE_ENV] ?? ""); if (!sourceKey) return false; - - const rawProvider = (process.env.NEMOCLAW_PROVIDER || "").trim().toLowerCase(); - const aliases = { - cloud: "build", - anthropiccompatible: "anthropicCompatible", - hermes: "hermesProvider", - "hermes-provider": "hermesProvider", - hermesprovider: "hermesProvider", - nous: "hermesProvider", - "nous-portal": "hermesProvider", - }; - const normalizedProvider = aliases[rawProvider] || rawProvider; + const normalizedProvider = normalizeNonInteractiveProviderKey(process.env.NEMOCLAW_PROVIDER); // ... }🤖 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 `@src/lib/onboard/providers.ts` around lines 221 - 231, Extract the provider-alias normalization into a single shared utility (e.g., export function normalizeProvider(raw?: string): string) and replace the inline alias map and normalization in both the current block (where rawProvider/aliases/normalizedProvider are defined) and in stageHostedInferenceSourceSecretEnv() and getNonInteractiveProvider() so they call normalizeProvider(process.env.NEMOCLAW_PROVIDER) (or normalizeProvider(raw) where raw is already passed); ensure the alias map (cloud -> build, anthropiccompatible -> anthropicCompatible, hermes/hermes-provider/hermesprovider/nous/ nous-portal -> hermesProvider, etc.) lives only in that utility and both call sites use its returned normalized value.
🤖 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 `@src/lib/onboard/providers.test.ts`:
- Around line 293-324: The test file currently only covers the auto-detect path
and the explicit cloud bypass; add additional test cases to exercise the other
hosted-routing branches so CI won't miss regressions: add one test that sets
NEMOCLAW_PROVIDER="custom" with a non-nvapi NVIDIA_INFERENCE_API_KEY and asserts
stageHostedInferenceSourceSecretEnv() returns true,
getRequestedProviderHint(true) returns "custom", and
process.env.COMPATIBLE_API_KEY/ NEMOCLAW_* vars are set to the hosted values
(mirror the first test but with NEMOCLAW_PROVIDER preset), and add another test
that sets NEMOCLAW_E2E_USE_HOSTED_INFERENCE="1" (with/without
NVIDIA_INFERENCE_API_KEY as appropriate) and asserts the same hosted-routing
outcomes; reference the helper functions stageHostedInferenceSourceSecretEnv,
getRequestedProviderHint, getRequestedModelHint and constants
HOSTED_INFERENCE_MODEL/HOSTED_INFERENCE_ENDPOINT_URL to implement the assertions
exactly as in the existing tests.
In `@src/lib/onboard/providers.ts`:
- Around line 221-231: Extract the provider-alias normalization into a single
shared utility (e.g., export function normalizeProvider(raw?: string): string)
and replace the inline alias map and normalization in both the current block
(where rawProvider/aliases/normalizedProvider are defined) and in
stageHostedInferenceSourceSecretEnv() and getNonInteractiveProvider() so they
call normalizeProvider(process.env.NEMOCLAW_PROVIDER) (or normalizeProvider(raw)
where raw is already passed); ensure the alias map (cloud -> build,
anthropiccompatible -> anthropicCompatible,
hermes/hermes-provider/hermesprovider/nous/ nous-portal -> hermesProvider, etc.)
lives only in that utility and both call sites use its returned normalized
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 97c8334d-0fd3-4455-aa43-60089e9df631
📒 Files selected for processing (12)
.github/workflows/nightly-e2e.yamlci/env-var-doc-allowlist.jsonsrc/lib/onboard/providers.test.tssrc/lib/onboard/providers.tstest/e2e-script-workflow.test.tstest/e2e/lib/ci-compatible-inference.shtest/e2e/test-agent-turn-latency-e2e.shtest/e2e/test-launchable-smoke.shtest/e2e/test-onboard-negative-paths.shtest/e2e/test-openclaw-skill-cli-e2e.shtest/e2e/test-skill-agent-e2e.shtest/onboard-selection.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/e2e/test-openclaw-skill-cli-e2e.sh
- test/e2e/test-skill-agent-e2e.sh
- test/e2e/test-onboard-negative-paths.sh
- .github/workflows/nightly-e2e.yaml
- test/e2e/lib/ci-compatible-inference.sh
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
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 @.github/workflows/nightly-e2e.yaml:
- Around line 1574-1579: The credential-migration Vitest lane is missing the
required hosted-mode flag; add the environment variable
NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1" to the job's env block that currently
contains NEMOCLAW_ENDPOINT_URL, NEMOCLAW_MODEL, NEMOCLAW_COMPAT_MODEL,
NVIDIA_INFERENCE_API_KEY and COMPATIBLE_API_KEY so this hosted-inference job
follows the same hosted code path as other nightly jobs and the
workflow-contract tests.
🪄 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: 6d4b3bc5-b731-4544-82c3-4cd347477306
📒 Files selected for processing (5)
.github/workflows/e2e-script.yaml.github/workflows/nightly-e2e.yamlsrc/lib/onboard/providers.test.tssrc/lib/onboard/providers.tstest/e2e-script-workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/onboard/providers.ts
Comment on lines
+1574
to
+1579
| NVIDIA_INFERENCE_API_KEY: ${{ (github.event_name != 'workflow_dispatch' || inputs.target_ref == '') && secrets.NVIDIA_INFERENCE_API_KEY || '' }} | ||
| 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 | ||
| COMPATIBLE_API_KEY: ${{ (github.event_name != 'workflow_dispatch' || inputs.target_ref == '') && secrets.NVIDIA_INFERENCE_API_KEY || '' }} |
Contributor
There was a problem hiding this comment.
Add the hosted-mode flag to the credential-migration Vitest lane.
This direct hosted-inference job is the only hosted-secret lane here that stages the custom endpoint env without NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1", so it can drift from the same hosted code path the other nightly jobs exercise. The current workflow-contract test also doesn't assert this flag, so the omission would keep slipping through.
Suggested fix
env:
NVIDIA_INFERENCE_API_KEY: ${{ (github.event_name != 'workflow_dispatch' || inputs.target_ref == '') && 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
COMPATIBLE_API_KEY: ${{ (github.event_name != 'workflow_dispatch' || inputs.target_ref == '') && secrets.NVIDIA_INFERENCE_API_KEY || '' }}As per coding guidelines, jobs that perform hosted inference must set NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1".
🤖 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/nightly-e2e.yaml around lines 1574 - 1579, The
credential-migration Vitest lane is missing the required hosted-mode flag; add
the environment variable NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1" to the job's env
block that currently contains NEMOCLAW_ENDPOINT_URL, NEMOCLAW_MODEL,
NEMOCLAW_COMPAT_MODEL, NVIDIA_INFERENCE_API_KEY and COMPATIBLE_API_KEY so this
hosted-inference job follows the same hosted code path as other nightly jobs and
the workflow-contract tests.
Source: Coding guidelines
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/lib/ci-compatible-inference.sh (1)
78-78: ⚡ Quick winTighten provider matching to avoid route-assertion false positives.
Line 78 currently allows prefix/superset matches (e.g.,
Provider: compatible-endpoint-v2would satisfy expectedcompatible-endpoint), which can hide routing regressions.Suggested patch
- grep -Eqi "Provider:[[:space:]]*${provider}" <<<"$plain" || return 1 + grep -Eqi "Provider:[[:space:]]*${provider}([[:space:]]|$)" <<<"$plain" || return 1🤖 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` at line 78, Tighten the provider-matching grep so it no longer treats longer names with the expected provider as a prefix: replace the pattern grep -Eqi "Provider:[[:space:]]*${provider}" <<<"$plain" || return 1 with a regex that requires the provider to be followed by end-of-line or whitespace (e.g., require (${provider} followed by $ or whitespace) ) so only exact/same-token matches succeed and prevent prefix/superset false positives.
🤖 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 `@test/e2e/lib/ci-compatible-inference.sh`:
- Line 78: Tighten the provider-matching grep so it no longer treats longer
names with the expected provider as a prefix: replace the pattern grep -Eqi
"Provider:[[:space:]]*${provider}" <<<"$plain" || return 1 with a regex that
requires the provider to be followed by end-of-line or whitespace (e.g., require
(${provider} followed by $ or whitespace) ) so only exact/same-token matches
succeed and prevent prefix/superset false positives.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 72f3fbf4-e24a-4174-9ad5-98d5a25bf87a
📒 Files selected for processing (6)
.github/workflows/nightly-e2e.yamltest/e2e/lib/ci-compatible-inference.shtest/e2e/test-agent-turn-latency-e2e.shtest/e2e/test-hermes-e2e.shtest/e2e/test-issue-4434-tui-unreachable-inference.shtest/e2e/test-launchable-smoke.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- test/e2e/test-agent-turn-latency-e2e.sh
- test/e2e/test-hermes-e2e.sh
- test/e2e/test-launchable-smoke.sh
- .github/workflows/nightly-e2e.yaml
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 27473531845
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27473849272
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27473928680
|
This was referenced Jun 13, 2026
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
1 participant
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
Route nightly hosted inference through NemoClaw's custom provider path instead of treating
NVIDIA_INFERENCE_API_KEYlike a Build/NVIDIA provider credential. The reusable and direct nightly E2E jobs now deriveCOMPATIBLE_API_KEYonly from the hosted source secret forhttps://inference-api.nvidia.com/v1.Changes
nvidia_secret_as_compatible_api_keycompatibility input and makenvidia_api_keyexport the hosted custom endpoint environment.NVIDIA_INFERENCE_API_KEYto providercustom, endpointhttps://inference-api.nvidia.com/v1, hosted Nemotron model, and derivedCOMPATIBLE_API_KEY.compatible-endpointin hosted CI mode while preserving non-hosted Build-provider validation.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit