ci(e2e): use canonical inference key in Vitest - #5688
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:
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdates hosted inference configuration, workflow env wiring, boundary validation, and live E2E scenarios to use ChangesE2E NVIDIA inference credential contract migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
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 |
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
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/e2e-scenarios/workflow-boundary.mts (1)
2479-2491: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate repeated NVIDIA secret checks in step validators.
These branches call
requireEnvDoesNotExposeSecret(..., "NVIDIA_INFERENCE_API_KEY")twice for the same step, which can emit duplicate boundary errors for a single violation.♻️ Suggested cleanup pattern
- requireEnvDoesNotExposeSecret( - errors, - stepName, - stepEnv, - "NVIDIA_INFERENCE_API_KEY", - ); requireEnvDoesNotExposeSecret( errors, stepName, stepEnv, "NVIDIA_INFERENCE_API_KEY", );Also applies to: 5221-5233, 5336-5347, 5503-5509, 6861-6872
🤖 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 2479 - 2491, Remove the duplicate call to requireEnvDoesNotExposeSecret that checks for "NVIDIA_INFERENCE_API_KEY" in the step validation logic. The function is being called twice consecutively with identical parameters within the same conditional block, which causes duplicate boundary errors when a violation occurs. Keep only one call to requireEnvDoesNotExposeSecret for this secret and apply the same deduplication fix to the other locations mentioned in the comment (around lines 5221-5233, 5336-5347, 5503-5509, and 6861-6872) where the same pattern of duplicate secret checks appears.
🤖 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 `@tools/e2e-scenarios/workflow-boundary.mts`:
- Around line 2479-2491: Remove the duplicate call to
requireEnvDoesNotExposeSecret that checks for "NVIDIA_INFERENCE_API_KEY" in the
step validation logic. The function is being called twice consecutively with
identical parameters within the same conditional block, which causes duplicate
boundary errors when a violation occurs. Keep only one call to
requireEnvDoesNotExposeSecret for this secret and apply the same deduplication
fix to the other locations mentioned in the comment (around lines 5221-5233,
5336-5347, 5503-5509, and 6861-6872) where the same pattern of duplicate secret
checks appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fbbf5254-abca-4c26-8571-cfcad696f5f0
📒 Files selected for processing (29)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/agent-turn-latency-helpers.tstest/e2e-scenario/live/brave-search-helpers.tstest/e2e-scenario/live/channels-add-remove.test.tstest/e2e-scenario/live/cloud-inference.test.tstest/e2e-scenario/live/common-egress-agent.test.tstest/e2e-scenario/live/cron-preflight-inference-local.test.tstest/e2e-scenario/live/device-auth-health-helpers.tstest/e2e-scenario/live/diagnostics.test.tstest/e2e-scenario/live/hermes-discord.test.tstest/e2e-scenario/live/hermes-inference-switch-helpers.tstest/e2e-scenario/live/messaging-providers-helpers.tstest/e2e-scenario/live/messaging-providers.test.tstest/e2e-scenario/live/openclaw-inference-switch.test.tstest/e2e-scenario/live/openclaw-skill-cli.test.tstest/e2e-scenario/live/phase6-messaging-helpers.tstest/e2e-scenario/live/sessions-agents-cli.test.tstest/e2e-scenario/live/snapshot-commands.test.tstest/e2e-scenario/live/spark-install.test.tstest/e2e-scenario/live/state-backup-restore.test.tstest/e2e-scenario/live/upgrade-stale-sandbox-helpers.tstest/e2e-scenario/support-tests/cloud-inference-provider-skip.test.tstest/e2e-scenario/support-tests/e2e-scenarios-workflow.test.tstest/e2e-scenario/support-tests/network-policy-transient-provider.test.tstest/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.tstest/e2e-scenario/support-tests/openclaw-slack-workflow-boundary.test.tstest/e2e-scenario/support-tests/spark-install-workflow-boundary.test.tstest/e2e-scenario/support-tests/tunnel-lifecycle-workflow-boundary.test.tstools/e2e-scenarios/workflow-boundary.mts
💤 Files with no reviewable changes (5)
- test/e2e-scenario/live/cron-preflight-inference-local.test.ts
- test/e2e-scenario/live/brave-search-helpers.ts
- test/e2e-scenario/support-tests/tunnel-lifecycle-workflow-boundary.test.ts
- test/e2e-scenario/support-tests/openclaw-slack-workflow-boundary.test.ts
- test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts
cv
approved these changes
Jun 23, 2026
Contributor
Author
Full Vitest run triage: inference key + Jetson runner blockersRun: https://github.com/NVIDIA/NemoClaw/actions/runs/28052592018 Findings from failed job logs:
Recommended next steps:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| async ({ artifacts, cleanup, host, sandbox, secrets, skip }) => { | ||
| const apiKey = await assertPrerequisites(host, secrets, skip); | ||
| const hosted = await assertPrerequisites(host, secrets, skip); | ||
| const apiKey = hosted.apiKey; |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| async ({ artifacts, cleanup, host, sandbox, secrets, skip }) => { | ||
| const apiKey = await assertPrerequisites(host, secrets, skip); | ||
| const hosted = await assertPrerequisites(host, secrets, skip); | ||
| const apiKey = hosted.apiKey; |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e-scenario/live/cloud-inference.test.ts (1)
251-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse helper-provided contract text instead of duplicating a literal.
This assertion should consume
hosted.contractLabelto keep boundary wording in one source of truth and avoid drift across scenarios.♻️ Proposed change
- "NVIDIA_INFERENCE_API_KEY is staged as the compatible endpoint credential", + hosted.contractLabel,🤖 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-scenario/live/cloud-inference.test.ts` at line 251, The assertion at line 251 uses a hardcoded literal string "NVIDIA_INFERENCE_API_KEY is staged as the compatible endpoint credential" instead of referencing the helper-provided contract text. Replace this hardcoded string with `hosted.contractLabel` to maintain a single source of truth for the boundary wording and prevent drift across scenarios.
🤖 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-scenario/live/common-egress-agent.test.ts`:
- Around line 265-271: Add a format validation check immediately after the
hosted variable is assigned from requireHostedInferenceConfig(secrets) call. The
validation should ensure the hosted key starts with the expected nvapi- prefix
format and throw or expect an error with a clear message if it does not match
the format. This fast-fail check should be placed before the return statement to
catch invalid key formats early with clear diagnostics rather than allowing them
to fail later during onboarding with less informative error messages.
---
Nitpick comments:
In `@test/e2e-scenario/live/cloud-inference.test.ts`:
- Line 251: The assertion at line 251 uses a hardcoded literal string
"NVIDIA_INFERENCE_API_KEY is staged as the compatible endpoint credential"
instead of referencing the helper-provided contract text. Replace this hardcoded
string with `hosted.contractLabel` to maintain a single source of truth for the
boundary wording and prevent drift across scenarios.
🪄 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: 3455e748-5a97-4f65-8d42-d86f118835cf
📒 Files selected for processing (15)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/fixtures/hosted-inference.tstest/e2e-scenario/live/cloud-inference.test.tstest/e2e-scenario/live/common-egress-agent.test.tstest/e2e-scenario/live/cron-preflight-inference-local.test.tstest/e2e-scenario/live/hermes-e2e.test.tstest/e2e-scenario/live/hermes-inference-switch-helpers.tstest/e2e-scenario/live/issue-4434-tui-unreachable-inference.test.tstest/e2e-scenario/live/launchable-smoke.test.tstest/e2e-scenario/live/openclaw-inference-switch.test.tstest/e2e-scenario/live/openclaw-skill-cli.test.tstest/e2e-scenario/live/sandbox-survival.test.tstest/e2e-scenario/live/shields-config.test.tstest/e2e-scenario/live/skill-agent.test.tstest/e2e-scenario/support-tests/hosted-inference.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e-scenario/live/openclaw-skill-cli.test.ts
Comment on lines
+265
to
+271
| const hosted = requireHostedInferenceConfig(secrets); | ||
| expect(process.env.NEMOCLAW_NON_INTERACTIVE, "NEMOCLAW_NON_INTERACTIVE=1 is required").toBe("1"); | ||
| expect( | ||
| process.env.NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE, | ||
| "NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 is required", | ||
| ).toBe("1"); | ||
| return apiKey; | ||
| return hosted; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a fast-fail hosted key format check in prerequisites.
At Line 265, requireHostedInferenceConfig guarantees presence but not nvapi- format; invalid secrets then fail deeper in onboarding with noisier diagnostics. Add a local format assertion here to fail immediately.
Suggested patch
const hosted = requireHostedInferenceConfig(secrets);
+ expect(
+ hosted.apiKey.startsWith("nvapi-"),
+ "NVIDIA_INFERENCE_API_KEY must start with `nvapi-` for hosted inference",
+ ).toBe(true);
expect(process.env.NEMOCLAW_NON_INTERACTIVE, "NEMOCLAW_NON_INTERACTIVE=1 is required").toBe("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-scenario/live/common-egress-agent.test.ts` around lines 265 - 271,
Add a format validation check immediately after the hosted variable is assigned
from requireHostedInferenceConfig(secrets) call. The validation should ensure
the hosted key starts with the expected nvapi- prefix format and throw or expect
an error with a clear message if it does not match the format. This fast-fail
check should be placed before the return statement to catch invalid key formats
early with clear diagnostics rather than allowing them to fail later during
onboarding with less informative error messages.
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28057277765
|
wscurran
added
area: ci
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28052592018
|
| assertTestOwnedSandboxName(); | ||
| const apiKey = secrets.required("NVIDIA_API_KEY"); | ||
| expect(apiKey.startsWith("nvapi-"), "NVIDIA_API_KEY must start with nvapi-").toBe(true); | ||
| const apiKey = secrets.required("NVIDIA_INFERENCE_API_KEY"); |
| assertTestOwnedSandboxName(); | ||
| const apiKey = secrets.required("NVIDIA_API_KEY"); | ||
| expect(apiKey.startsWith("nvapi-"), "NVIDIA_API_KEY must start with nvapi-").toBe(true); | ||
| const apiKey = secrets.required("NVIDIA_INFERENCE_API_KEY"); |
5 tasks
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28176166049
|
This was referenced Jun 25, 2026
21 tasks
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary - Route Vitest E2E workflow secret wiring through NVIDIA_INFERENCE_API_KEY only. - Update Vitest live tests/helpers to require/stage NVIDIA_INFERENCE_API_KEY instead of NVIDIA_API_KEY. - Update workflow boundary guards so Vitest jobs no longer depend on secrets.NVIDIA_API_KEY. ## Scope Vitest-only: .github/workflows/e2e-vitest-scenarios.yaml, test/e2e-scenario/**, and tools/e2e-scenarios/workflow-boundary.mts. ## Validation - npm test -- test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/tunnel-lifecycle-workflow-boundary.test.ts test/e2e-scenario/support-tests/spark-install-workflow-boundary.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts test/e2e-scenario/support-tests/openclaw-slack-workflow-boundary.test.ts test/e2e-scenario/support-tests/cloud-inference-provider-skip.test.ts test/e2e-scenario/support-tests/network-policy-transient-provider.test.ts - git diff --check - rg NVIDIA_API_KEY .github/workflows/e2e-vitest-scenarios.yaml test/e2e-scenario tools/e2e-scenarios (no matches) Note: local all-test hooks are currently blocked by pre-existing local fixture/build issues (missing nemoclaw/dist and nemoclaw/node_modules/json5 in this worktree), so the commit/push used targeted validation for this Vitest-only change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Chores** * Updated E2E/Vitest GitHub workflow and scenario boundary validations to use `NVIDIA_INFERENCE_API_KEY` (and to unset it where appropriate) instead of `NVIDIA_API_KEY`. * Enabled hosted-inference mode (`NEMOCLAW_E2E_USE_HOSTED_INFERENCE=1`) and standardized how hosted inference configuration is propagated into jobs and steps. * **Tests** * Refreshed live E2E scenarios and helpers to source inference credentials via hosted-inference configuration, updating credential redaction/leak checks and expected boundary/error messages accordingly. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
4 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
Scope
Vitest-only: .github/workflows/e2e-vitest-scenarios.yaml, test/e2e-scenario/**, and tools/e2e-scenarios/workflow-boundary.mts.
Validation
Note: local all-test hooks are currently blocked by pre-existing local fixture/build issues (missing nemoclaw/dist and nemoclaw/node_modules/json5 in this worktree), so the commit/push used targeted validation for this Vitest-only change.
Summary by CodeRabbit
Release Notes
Chores
NVIDIA_INFERENCE_API_KEY(and to unset it where appropriate) instead ofNVIDIA_API_KEY.NEMOCLAW_E2E_USE_HOSTED_INFERENCE=1) and standardized how hosted inference configuration is propagated into jobs and steps.Tests