fix(ci): wire 6 unwired E2E scripts into nightly pipeline - #2607
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request adds six new nightly E2E workflow jobs to the GitHub Actions workflow file. Four jobs ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-e2e.yaml:
- Around line 605-646: The two jobs credential-sanitization-e2e and
telegram-injection-e2e assume a pre-existing sandbox but lack dependencies or
sandbox bootstrapping; add either a needs: reference to the job that creates the
sandbox (e.g., needs: [test-full-e2e-setup] or the existing sandbox-creation job
name) or add steps to bootstrap a sandbox in each job (set
NEMOCLAW_SANDBOX_NAME, run the install/onboard/bootstrap script before running
test-*.sh), and ensure the environment variable NEMOCLAW_SANDBOX_NAME is
explicitly set in each job so sandbox_exec() targets the correct sandbox.
🪄 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: c3604c91-bc92-4592-86c0-6668cf01140a
📒 Files selected for processing (1)
.github/workflows/nightly-e2e.yaml
a6412bb to
2249e94
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test-security-e2e-manual.yaml:
- Around line 17-24: The two workflow steps use the same NEMOCLAW_SANDBOX_NAME
("e2e-test"), causing sandbox name collisions; change each job's
NEMOCLAW_SANDBOX_NAME to be unique (e.g., include the job name or run id) so
parallel jobs don't race — update the occurrences referenced by the "Run
credential sanitization E2E test" job and the other E2E job to values like
"e2e-test-credential-sanitization" or "e2e-test-${{ github.run_id }}" (or
similar job-specific suffix) so each job creates/uses a distinct sandbox.
🪄 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: 6c188b70-fb8e-4028-8035-8153ce435a06
📒 Files selected for processing (2)
.github/workflows/test-onboard-e2e-manual.yaml.github/workflows/test-security-e2e-manual.yaml
| NEMOCLAW_SANDBOX_NAME: "e2e-test" | ||
| run: bash install.sh --non-interactive --yes-i-accept-third-party-software | ||
| - name: Run credential sanitization E2E test | ||
| env: | ||
| NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} | ||
| NEMOCLAW_NON_INTERACTIVE: "1" | ||
| NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" | ||
| NEMOCLAW_SANDBOX_NAME: "e2e-test" |
There was a problem hiding this comment.
Use distinct sandbox names per security job to avoid cross-job collisions.
Line 17/24 and Line 47/54 use the same NEMOCLAW_SANDBOX_NAME (e2e-test). These jobs run independently and can execute in parallel, which can race on the same remote sandbox and cause flaky failures.
🔧 Proposed fix
credential-sanitization-e2e:
runs-on: ubuntu-latest
timeout-minutes: 60
@@
- name: Install NemoClaw and onboard sandbox
env:
NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }}
NEMOCLAW_NON_INTERACTIVE: "1"
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1"
- NEMOCLAW_SANDBOX_NAME: "e2e-test"
+ NEMOCLAW_SANDBOX_NAME: "e2e-security-credential-${{ github.run_id }}"
@@
- name: Run credential sanitization E2E test
env:
NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }}
NEMOCLAW_NON_INTERACTIVE: "1"
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1"
- NEMOCLAW_SANDBOX_NAME: "e2e-test"
+ NEMOCLAW_SANDBOX_NAME: "e2e-security-credential-${{ github.run_id }}"
@@
telegram-injection-e2e:
runs-on: ubuntu-latest
timeout-minutes: 60
@@
- name: Install NemoClaw and onboard sandbox
env:
NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }}
NEMOCLAW_NON_INTERACTIVE: "1"
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1"
- NEMOCLAW_SANDBOX_NAME: "e2e-test"
+ NEMOCLAW_SANDBOX_NAME: "e2e-security-telegram-${{ github.run_id }}"
@@
- name: Run telegram injection E2E test
env:
NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }}
NEMOCLAW_NON_INTERACTIVE: "1"
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1"
- NEMOCLAW_SANDBOX_NAME: "e2e-test"
+ NEMOCLAW_SANDBOX_NAME: "e2e-security-telegram-${{ github.run_id }}"Also applies to: 47-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test-security-e2e-manual.yaml around lines 17 - 24, The
two workflow steps use the same NEMOCLAW_SANDBOX_NAME ("e2e-test"), causing
sandbox name collisions; change each job's NEMOCLAW_SANDBOX_NAME to be unique
(e.g., include the job name or run id) so parallel jobs don't race — update the
occurrences referenced by the "Run credential sanitization E2E test" job and the
other E2E job to values like "e2e-test-credential-sanitization" or "e2e-test-${{
github.run_id }}" (or similar job-specific suffix) so each job creates/uses a
distinct sandbox.
4 self-contained scripts (NVIDIA#2566): - test-double-onboard.sh (lifecycle recovery, multi-sandbox) - test-onboard-repair.sh (resume repair, invalidation) - test-onboard-resume.sh (interrupted onboard -> resume) - test-runtime-overrides.sh (model override, CORS config) 2 security scripts (NVIDIA#2567): - test-credential-sanitization.sh (credential stripping, digest) - test-telegram-injection.sh (command injection prevention) Each job bootstraps its own sandbox via install.sh before running the test script. All 6 wired into notify-on-failure. Fixes NVIDIA#2566 Fixes NVIDIA#2567 Signed-off-by: Truong Nguyen <tgnguyen@nvidia.com> Made-with: Cursor
59fc4fd to
5f2204d
Compare
## Summary The 6 E2E jobs wired in #2607 (`double-onboard-e2e`, `onboard-repair-e2e`, `onboard-resume-e2e`, `runtime-overrides-e2e`, `credential-sanitization-e2e`, `telegram-injection-e2e`) were added without the selective dispatch guard, breaking `validate-e2e-coverage.test.ts` on main. ## Changes - Adds the standard dispatch guard to all 6 jobs: ```yaml if: >- github.repository == 'NVIDIA/NemoClaw' && (github.event_name != 'workflow_dispatch' || inputs.jobs == '' || contains(format(',{0},', inputs.jobs), ',<job-name>,')) ``` - Adds all 6 job names to the `workflow_dispatch.inputs.jobs` description ## Why this matters - **main is broken** — the `checks` CI job fails on every PR due to this - Selective dispatch (`-f jobs=cloud-e2e`) currently runs these 6 jobs unconditionally ## Type of Change - Code change (bug fix) ## Verification - YAML validated locally - `validate-e2e-coverage.test.ts` should pass with these guards in place <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI/CD testing automation to enhance selective E2E test execution. The workflow now supports independent execution of specific test scenarios—including double-onboard, onboard-repair, onboard-resume, runtime-overrides, credential-sanitization, and telegram-injection tests—alongside full test suite options, configurable based on workflow dispatch selections. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
…d-resume tests (#2657) ## Summary Fixes the persistent `onboard-repair-e2e` and `onboard-resume-e2e` nightly failures by replacing a broken fault injection mechanism. **Zero product code changes** — test-only fix. ## Problem Both tests create interrupted onboard state by setting `NEMOCLAW_POLICY_MODE=invalid`, expecting onboard to `process.exit(1)` at the policy step. PR #2434 (merged Apr 24) intentionally changed this behavior: invalid policy modes now `console.warn()` and fall back to suggested presets instead of hard-failing. Onboard completes successfully (exit 0) where the tests expect exit 1. Both tests have failed in **every nightly run** since PR #2607 wired them into the pipeline on Apr 28. ## Fix Replace `NEMOCLAW_POLICY_MODE=invalid` with `NEMOCLAW_POLICY_MODE=custom` (without setting `NEMOCLAW_POLICY_PRESETS`). This uses a **real, stable validation path** in `setupPoliciesWithSelection()`: ```typescript if (policyMode === "custom" || policyMode === "list") { chosen = parsePolicyPresetEnv(process.env.NEMOCLAW_POLICY_PRESETS || ""); if (chosen.length === 0) { console.error(" NEMOCLAW_POLICY_PRESETS is required when NEMOCLAW_POLICY_MODE=custom."); process.exit(1); // ← still a hard exit, as it should be } } ``` This is correct product behavior (custom mode requires presets), and it triggers the same `process.exit(1)` → session `markStepFailed()` flow the tests need. ## Changes - `test/e2e/test-onboard-repair.sh`: Replace `NEMOCLAW_POLICY_MODE=invalid` → `NEMOCLAW_POLICY_MODE=custom` + `NEMOCLAW_POLICY_PRESETS=""`, update assertion - `test/e2e/test-onboard-resume.sh`: Same change ## Testing These tests run as `onboard-repair-e2e` and `onboard-resume-e2e` in the nightly pipeline. Requesting a nightly dispatch to validate. ## Related - #2573 — ci(nightly-e2e): add onboard negative-path E2E test (tracks this fix) - #2434 — fix(onboard): fall back to tier suggestions on bad NEMOCLAW_POLICY_MODE (the change that broke the old injection) - #2607 — fix(ci): wire 6 unwired E2E scripts into nightly pipeline (wired these tests) - #446 — nemoclaw onboard is not resumable (consolidated into #2573, now closed) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end flows to induce policy-step validation via a custom mode with empty presets and expect the corresponding validation message. * Broadened resume verification to accept either re-running or explicitly skipping inference. * Made sandbox-creation checks more generic. * Added an extra interrupted re-creation run to exercise conflict-detection on resume. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Wire 6 existing E2E test scripts into `nightly-e2e.yaml` that have never been part of any automated CI job despite being self-contained and actively maintained. ## Related Issue Fixes NVIDIA#2566 Fixes NVIDIA#2567 ## Changes **4 self-contained scripts (NVIDIA#2566):** - `test-double-onboard.sh` — lifecycle recovery, repeat onboard reuses gateway, multi-sandbox coexistence - `test-onboard-repair.sh` — resume recreates missing sandbox, rejects conflicting name/provider/model - `test-onboard-resume.sh` — interrupted onboard → resume → verify completion - `test-runtime-overrides.sh` — NEMOCLAW_MODEL_OVERRIDE, CORS config, env-driven sandbox patching **2 security scripts (NVIDIA#2567):** - `test-credential-sanitization.sh` — credential stripping from migration snapshots, auth-profiles.json deletion, blueprint digest verification, symlink traversal - `test-telegram-injection.sh` — command injection prevention through Telegram bridge message handling All 6 scripts already exist in `test/e2e/`, were created March–April, and are actively maintained (last touched Apr 23 for e2e-timeout refactor). Each job has artifact upload on failure and is wired into `notify-on-failure`. ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - YAML validated: all 25 jobs (19 existing + 6 new) parse correctly on fork dispatch - No test script changes — only workflow wiring ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen <tgnguyen@nvidia.com> Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Added six nightly end-to-end test jobs to broaden automated E2E coverage across onboarding, runtime overrides, credential sanitization, and injection scenarios. * Nightly jobs upload logs on failure and are included in centralized failure/cancel notifications. * Added manually-triggered E2E workflows for onboarding and security tests, each providing targeted test runs and conditional log uploads on failure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Truong Nguyen <tgnguyen@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
…NVIDIA#2637) ## Summary The 6 E2E jobs wired in NVIDIA#2607 (`double-onboard-e2e`, `onboard-repair-e2e`, `onboard-resume-e2e`, `runtime-overrides-e2e`, `credential-sanitization-e2e`, `telegram-injection-e2e`) were added without the selective dispatch guard, breaking `validate-e2e-coverage.test.ts` on main. ## Changes - Adds the standard dispatch guard to all 6 jobs: ```yaml if: >- github.repository == 'NVIDIA/NemoClaw' && (github.event_name != 'workflow_dispatch' || inputs.jobs == '' || contains(format(',{0},', inputs.jobs), ',<job-name>,')) ``` - Adds all 6 job names to the `workflow_dispatch.inputs.jobs` description ## Why this matters - **main is broken** — the `checks` CI job fails on every PR due to this - Selective dispatch (`-f jobs=cloud-e2e`) currently runs these 6 jobs unconditionally ## Type of Change - Code change (bug fix) ## Verification - YAML validated locally - `validate-e2e-coverage.test.ts` should pass with these guards in place <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI/CD testing automation to enhance selective E2E test execution. The workflow now supports independent execution of specific test scenarios—including double-onboard, onboard-repair, onboard-resume, runtime-overrides, credential-sanitization, and telegram-injection tests—alongside full test suite options, configurable based on workflow dispatch selections. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
…d-resume tests (NVIDIA#2657) ## Summary Fixes the persistent `onboard-repair-e2e` and `onboard-resume-e2e` nightly failures by replacing a broken fault injection mechanism. **Zero product code changes** — test-only fix. ## Problem Both tests create interrupted onboard state by setting `NEMOCLAW_POLICY_MODE=invalid`, expecting onboard to `process.exit(1)` at the policy step. PR NVIDIA#2434 (merged Apr 24) intentionally changed this behavior: invalid policy modes now `console.warn()` and fall back to suggested presets instead of hard-failing. Onboard completes successfully (exit 0) where the tests expect exit 1. Both tests have failed in **every nightly run** since PR NVIDIA#2607 wired them into the pipeline on Apr 28. ## Fix Replace `NEMOCLAW_POLICY_MODE=invalid` with `NEMOCLAW_POLICY_MODE=custom` (without setting `NEMOCLAW_POLICY_PRESETS`). This uses a **real, stable validation path** in `setupPoliciesWithSelection()`: ```typescript if (policyMode === "custom" || policyMode === "list") { chosen = parsePolicyPresetEnv(process.env.NEMOCLAW_POLICY_PRESETS || ""); if (chosen.length === 0) { console.error(" NEMOCLAW_POLICY_PRESETS is required when NEMOCLAW_POLICY_MODE=custom."); process.exit(1); // ← still a hard exit, as it should be } } ``` This is correct product behavior (custom mode requires presets), and it triggers the same `process.exit(1)` → session `markStepFailed()` flow the tests need. ## Changes - `test/e2e/test-onboard-repair.sh`: Replace `NEMOCLAW_POLICY_MODE=invalid` → `NEMOCLAW_POLICY_MODE=custom` + `NEMOCLAW_POLICY_PRESETS=""`, update assertion - `test/e2e/test-onboard-resume.sh`: Same change ## Testing These tests run as `onboard-repair-e2e` and `onboard-resume-e2e` in the nightly pipeline. Requesting a nightly dispatch to validate. ## Related - NVIDIA#2573 — ci(nightly-e2e): add onboard negative-path E2E test (tracks this fix) - NVIDIA#2434 — fix(onboard): fall back to tier suggestions on bad NEMOCLAW_POLICY_MODE (the change that broke the old injection) - NVIDIA#2607 — fix(ci): wire 6 unwired E2E scripts into nightly pipeline (wired these tests) - NVIDIA#446 — nemoclaw onboard is not resumable (consolidated into NVIDIA#2573, now closed) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end flows to induce policy-step validation via a custom mode with empty presets and expect the corresponding validation message. * Broadened resume verification to accept either re-running or explicitly skipping inference. * Made sandbox-creation checks more generic. * Added an extra interrupted re-creation run to exercise conflict-detection on resume. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Wire 6 existing E2E test scripts into
nightly-e2e.yamlthat have never been part of any automated CI job despite being self-contained and actively maintained.Related Issue
Fixes #2566
Fixes #2567
Changes
4 self-contained scripts (#2566):
test-double-onboard.sh— lifecycle recovery, repeat onboard reuses gateway, multi-sandbox coexistencetest-onboard-repair.sh— resume recreates missing sandbox, rejects conflicting name/provider/modeltest-onboard-resume.sh— interrupted onboard → resume → verify completiontest-runtime-overrides.sh— NEMOCLAW_MODEL_OVERRIDE, CORS config, env-driven sandbox patching2 security scripts (#2567):
test-credential-sanitization.sh— credential stripping from migration snapshots, auth-profiles.json deletion, blueprint digest verification, symlink traversaltest-telegram-injection.sh— command injection prevention through Telegram bridge message handlingAll 6 scripts already exist in
test/e2e/, were created March–April, and are actively maintained (last touched Apr 23 for e2e-timeout refactor). Each job has artifact upload on failure and is wired intonotify-on-failure.Type of Change
Verification
AI Disclosure
Signed-off-by: Truong Nguyen tgnguyen@nvidia.com
Made with Cursor
Summary by CodeRabbit