test(e2e): hermeticize onboard resume - #5702
Merged
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:
📝 WalkthroughWalkthroughThe Changesonboard-resume-e2e Hermetic Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
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 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
Selective E2E Results —
|
| Job | Result |
|---|---|
| onboard-resume-e2e |
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: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e-script-workflow.test.ts (1)
1035-1041: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winStrengthen hermiticity assertions for secret/env routing.
Line 1036 verifies
-u NVIDIA_INFERENCE_API_KEY, but the contract also depends on unsettingGITHUB_TOKENand keeping hosted-inference env knobs off the run step. Add explicit assertions so regressions are caught immediately.Suggested test hardening
expect(installOpenShellStep?.run).toContain("scripts/install-openshell.sh"); expect(installOpenShellStep?.run).toContain("-u NVIDIA_INFERENCE_API_KEY"); + expect(installOpenShellStep?.run).toContain("-u GITHUB_TOKEN"); expect(runStep?.env?.NVIDIA_INFERENCE_API_KEY).toBeUndefined(); expect(runStep?.env?.COMPATIBLE_API_KEY).toBeUndefined(); expect(runStep?.env?.NEMOCLAW_ENDPOINT_URL).toBeUndefined(); expect(runStep?.env?.NEMOCLAW_PROVIDER).toBeUndefined(); + expect(runStep?.env?.NEMOCLAW_E2E_USE_HOSTED_INFERENCE).toBeUndefined(); + expect(runStep?.env?.NEMOCLAW_MODEL).toBeUndefined(); + expect(runStep?.env?.NEMOCLAW_COMPAT_MODEL).toBeUndefined(); + expect(runStep?.env?.NEMOCLAW_PREFERRED_API).toBeUndefined();As per path instructions, the onboard-resume job must avoid hosted-inference secrets/env wiring and preserve hermetic local-endpoint behavior.
🤖 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 1035 - 1041, The test assertions for the onboard-resume job need strengthening to catch regressions in secret and environment variable routing. In addition to the existing assertion that checks for "-u NVIDIA_INFERENCE_API_KEY" in the installOpenShellStep?.run, add an explicit assertion to verify that "-u GITHUB_TOKEN" is also present in the installOpenShellStep?.run to ensure the script is unsetting the GitHub token. Additionally, add explicit assertions to verify that any hosted-inference related environment variables are not present on the runStep?.env object to maintain hermetic local-endpoint behavior and prevent accidental wiring of hosted-inference secrets.Source: Path instructions
🤖 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-script-workflow.test.ts`:
- Around line 1035-1041: The test assertions for the onboard-resume job need
strengthening to catch regressions in secret and environment variable routing.
In addition to the existing assertion that checks for "-u
NVIDIA_INFERENCE_API_KEY" in the installOpenShellStep?.run, add an explicit
assertion to verify that "-u GITHUB_TOKEN" is also present in the
installOpenShellStep?.run to ensure the script is unsetting the GitHub token.
Additionally, add explicit assertions to verify that any hosted-inference
related environment variables are not present on the runStep?.env object to
maintain hermetic local-endpoint behavior and prevent accidental wiring of
hosted-inference secrets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 55b28b1e-cbd0-49f7-a92d-e572335d0c66
📒 Files selected for processing (3)
.github/workflows/nightly-e2e.yamltest/e2e-script-workflow.test.tstest/e2e/test-onboard-resume.sh
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28066708126
|
wscurran
added
area: e2e
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28122464627
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28136867216
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28141810361
|
5 tasks
cv
approved these changes
Jun 25, 2026
4 tasks
5 tasks
jyaunches
added a commit
that referenced
this pull request
Jun 26, 2026
## Summary Restore issue #5849 parity package `Package A` for merged bash-suite deltas from #5702 only. This moves the `onboard-resume-vitest` path to the same hermetic fake OpenAI-compatible contract as the late bash guard: - no `NVIDIA_INFERENCE_API_KEY` / hosted `COMPATIBLE_API_KEY` workflow dependency - local fake OpenAI-compatible endpoint started by the Vitest - authenticated fake endpoint request capture asserted - resume runs with both hosted secret env vars absent and must hydrate from gateway/session state ## Related Issues Refs #5849 Refs #5702 ## Scope gate - Package: `Package A` - Included PRs all merged and touched `test/e2e`: yes — #5702 touched `test/e2e/README.md`, `test/e2e/lib/hermetic-compatible-inference.sh`, `test/e2e/test-onboard-resume.sh` - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | A1 | #5702 | Onboard-resume uses a local fake OpenAI-compatible endpoint instead of hosted NVIDIA inference secrets. | `hermetic-default` | `test/e2e-scenario/live/onboard-resume.test.ts` starts `startFakeOpenAiCompatibleServer`, sets `NEMOCLAW_PROVIDER=custom` + `COMPATIBLE_API_KEY` only for first onboard, and asserts fake authenticated inference requests. | covered | | A2 | #5702 | Resume run removes both `NVIDIA_INFERENCE_API_KEY` and `COMPATIBLE_API_KEY`, proving credential recovery comes from gateway/session state. | `hermetic-default` | `test/e2e-scenario/live/onboard-resume.test.ts` builds resume env from fixture allowlist and asserts both secret env vars are undefined. | covered | | A3 | #5702 | Selective Vitest workflow routing must not inject hosted inference secrets into onboard-resume. | `hermetic-default` | `.github/workflows/e2e-vitest-scenarios.yaml` removes hosted env/model wiring; `test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts` guards the workflow shape. | covered | | A4 | #5702 | Legacy bash workflow remains hermetic until shell retirement. | `hermetic-default` | Existing `test/e2e-script-workflow.test.ts` guard still covers `nightly-e2e.yaml` / bash lane shape. | already covered | ## Inference mode support - Default mode for touched live targets: `mock/hermetic` - Real inference support preserved: not applicable; this package explicitly removes hosted inference dependency for onboard-resume - Modes validated in this PR: hermetic/source-shape locally; selective `onboard-resume-vitest` workflow required for full live Docker/OpenShell execution - If not validated with real inference: real inference is not required by `hermetic-default`; the regression contract is fake-compatible auth/request capture without hosted secrets ## Validation - [x] `git diff --check` - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts` - [x] `npx tsc --noEmit --allowImportingTsExtensions --module ESNext --moduleResolution Bundler --target ES2022 --types node,vitest/globals --skipLibCheck test/e2e-scenario/live/onboard-resume.test.ts` - [x] `npx vitest run test/e2e-script-workflow.test.ts` - [x] selective `onboard-resume-vitest` workflow: https://github.com/NVIDIA/NemoClaw/actions/runs/28241580508 ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved the resume onboarding flow to work with a local compatible inference endpoint instead of relying on hosted credentials. * Tightened credential handling so sensitive keys are no longer passed into the related setup and test steps. * Updated end-to-end coverage to verify the workflow runs without injecting inference secrets and still completes successfully. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary - convert onboard-resume-e2e from hosted NVIDIA inference to a local fake OpenAI-compatible endpoint - build/install the source CLI + OpenShell in the nightly job instead of running install.sh auto-onboard against hosted inference - add workflow contract coverage that onboard-resume no longer consumes NVIDIA_INFERENCE_API_KEY or COMPATIBLE_API_KEY from GitHub secrets - establish the reusable `test/e2e/lib/hermetic-compatible-inference.sh` pattern for follow-on issue NVIDIA#5747 direct bash conversions ## Test plan - git diff --check - bash -n test/e2e/test-onboard-resume.sh - npm run source-shape:check - npm test -- test/e2e-advisor-dispatch.test.ts test/e2e-script-workflow.test.ts - npm run build:cli - bash -n test/e2e/lib/hermetic-compatible-inference.sh Note: full pre-commit Test (CLI) was skipped for commit because the local full-suite run currently fails on unrelated local environment/setup issues (missing nemoclaw/node_modules/json5, ssrf-parity plugin dist, one local timeout). Targeted workflow-contract validation passed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Tests** * Updated onboard-resume E2E to run against a locally reachable OpenAI-compatible fake endpoint with stronger authenticated-request validation and improved setup/teardown. * Tightened hermeticity checks to ensure hosted-inference-related configuration is not used. * Updated workflow-step routing expectations and added new assertions covering the hermetic compatible-inference flow. * **Documentation** * Added guidance for choosing hermetic compatible inference helpers and updated script prerequisites (no longer requires setting an NVIDIA inference key). * **CI/Workflow** * Reordered the nightly onboard-resume E2E job to use a Node-based build/CLI setup flow before running the E2E tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Restore issue NVIDIA#5849 parity package `Package A` for merged bash-suite deltas from NVIDIA#5702 only. This moves the `onboard-resume-vitest` path to the same hermetic fake OpenAI-compatible contract as the late bash guard: - no `NVIDIA_INFERENCE_API_KEY` / hosted `COMPATIBLE_API_KEY` workflow dependency - local fake OpenAI-compatible endpoint started by the Vitest - authenticated fake endpoint request capture asserted - resume runs with both hosted secret env vars absent and must hydrate from gateway/session state ## Related Issues Refs NVIDIA#5849 Refs NVIDIA#5702 ## Scope gate - Package: `Package A` - Included PRs all merged and touched `test/e2e`: yes — NVIDIA#5702 touched `test/e2e/README.md`, `test/e2e/lib/hermetic-compatible-inference.sh`, `test/e2e/test-onboard-resume.sh` - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | A1 | NVIDIA#5702 | Onboard-resume uses a local fake OpenAI-compatible endpoint instead of hosted NVIDIA inference secrets. | `hermetic-default` | `test/e2e-scenario/live/onboard-resume.test.ts` starts `startFakeOpenAiCompatibleServer`, sets `NEMOCLAW_PROVIDER=custom` + `COMPATIBLE_API_KEY` only for first onboard, and asserts fake authenticated inference requests. | covered | | A2 | NVIDIA#5702 | Resume run removes both `NVIDIA_INFERENCE_API_KEY` and `COMPATIBLE_API_KEY`, proving credential recovery comes from gateway/session state. | `hermetic-default` | `test/e2e-scenario/live/onboard-resume.test.ts` builds resume env from fixture allowlist and asserts both secret env vars are undefined. | covered | | A3 | NVIDIA#5702 | Selective Vitest workflow routing must not inject hosted inference secrets into onboard-resume. | `hermetic-default` | `.github/workflows/e2e-vitest-scenarios.yaml` removes hosted env/model wiring; `test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts` guards the workflow shape. | covered | | A4 | NVIDIA#5702 | Legacy bash workflow remains hermetic until shell retirement. | `hermetic-default` | Existing `test/e2e-script-workflow.test.ts` guard still covers `nightly-e2e.yaml` / bash lane shape. | already covered | ## Inference mode support - Default mode for touched live targets: `mock/hermetic` - Real inference support preserved: not applicable; this package explicitly removes hosted inference dependency for onboard-resume - Modes validated in this PR: hermetic/source-shape locally; selective `onboard-resume-vitest` workflow required for full live Docker/OpenShell execution - If not validated with real inference: real inference is not required by `hermetic-default`; the regression contract is fake-compatible auth/request capture without hosted secrets ## Validation - [x] `git diff --check` - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts` - [x] `npx tsc --noEmit --allowImportingTsExtensions --module ESNext --moduleResolution Bundler --target ES2022 --types node,vitest/globals --skipLibCheck test/e2e-scenario/live/onboard-resume.test.ts` - [x] `npx vitest run test/e2e-script-workflow.test.ts` - [x] selective `onboard-resume-vitest` workflow: https://github.com/NVIDIA/NemoClaw/actions/runs/28241580508 ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved the resume onboarding flow to work with a local compatible inference endpoint instead of relying on hosted credentials. * Tightened credential handling so sensitive keys are no longer passed into the related setup and test steps. * Updated end-to-end coverage to verify the workflow runs without injecting inference secrets and still completes successfully. <!-- 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
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
test/e2e/lib/hermetic-compatible-inference.shpattern for follow-on issue test(e2e): hermeticize bash E2E inference to reduce nightly 429s #5747 direct bash conversionsTest plan
Note: full pre-commit Test (CLI) was skipped for commit because the local full-suite run currently fails on unrelated local environment/setup issues (missing nemoclaw/node_modules/json5, ssrf-parity plugin dist, one local timeout). Targeted workflow-contract validation passed.
Summary by CodeRabbit
Release Notes
Tests
Documentation
CI/Workflow