fix(e2e): replace broken fault injection in onboard-repair and onboard-resume tests - #2657
Merged
Conversation
…d-resume tests PR #2434 changed invalid NEMOCLAW_POLICY_MODE values from process.exit(1) to a graceful console.warn() fallback. This broke the fault injection mechanism in test-onboard-repair.sh and test-onboard-resume.sh, which relied on NEMOCLAW_POLICY_MODE=invalid to force a policy-step failure and create interrupted session state for resume/repair testing. Replace with NEMOCLAW_POLICY_MODE=custom (without NEMOCLAW_POLICY_PRESETS), which is a real validation path that still exits 1 at the policy step. Fixes #2573
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR updates two e2e test scripts to change how Phase 2 policy-step failures are induced (use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25088948895
|
![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/test-onboard-repair.sh (1)
137-143: Consider extracting shared Phase-2 fault-injection setup into a helper.
test-onboard-repair.shandtest-onboard-resume.shnow duplicate the same interruption setup/comments; centralizing this would reduce future drift when onboard messaging or env behavior changes.Also applies to: 149-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/test-onboard-repair.sh` around lines 137 - 143, Extract the duplicated Phase-2 fault-injection setup and its explanatory comments from test-onboard-repair.sh and test-onboard-resume.sh into a single helper (e.g., an executable script or sourced function named setup_phase2_fault_injection or e2e_onboard_fault_helper) that exports the same environment variables (NEMOCLAW_POLICY_MODE=custom and the related settings) and documents the rationale; then replace the duplicated blocks in both test-onboard-repair.sh and test-onboard-resume.sh with a single call/source to that helper so both tests share the same implementation and comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/test-onboard-repair.sh`:
- Around line 137-143: Extract the duplicated Phase-2 fault-injection setup and
its explanatory comments from test-onboard-repair.sh and test-onboard-resume.sh
into a single helper (e.g., an executable script or sourced function named
setup_phase2_fault_injection or e2e_onboard_fault_helper) that exports the same
environment variables (NEMOCLAW_POLICY_MODE=custom and the related settings) and
documents the rationale; then replace the duplicated blocks in both
test-onboard-repair.sh and test-onboard-resume.sh with a single call/source to
that helper so both tests share the same implementation and comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 606ea559-fcf9-428b-aad9-ce1c2e2d8cd7
📒 Files selected for processing (2)
test/e2e/test-onboard-repair.shtest/e2e/test-onboard-resume.sh
…rd-repair - onboard-resume: inference step is skipped (not re-run) on resume when already configured; accept both [resume] Skipping and [4/7] patterns - onboard-repair: step numbering is [6/8] not [5/7]; match 'Creating sandbox' without hardcoded step number - onboard-repair: re-create interrupted session state before Phase 4/5 conflict tests, since Phase 3's successful resume sets resumable=false
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25089651902
|
cv
approved these changes
Apr 29, 2026
DemianHeyGen
pushed a commit
to DemianHeyGen/NemoClaw
that referenced
this pull request
Apr 30, 2026
…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 -->
wscurran
added
area: ci
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
Fixes the persistent
onboard-repair-e2eandonboard-resume-e2enightly 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 toprocess.exit(1)at the policy step. PR #2434 (merged Apr 24) intentionally changed this behavior: invalid policy modes nowconsole.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=invalidwithNEMOCLAW_POLICY_MODE=custom(without settingNEMOCLAW_POLICY_PRESETS). This uses a real, stable validation path insetupPoliciesWithSelection():This is correct product behavior (custom mode requires presets), and it triggers the same
process.exit(1)→ sessionmarkStepFailed()flow the tests need.Changes
test/e2e/test-onboard-repair.sh: ReplaceNEMOCLAW_POLICY_MODE=invalid→NEMOCLAW_POLICY_MODE=custom+NEMOCLAW_POLICY_PRESETS="", update assertiontest/e2e/test-onboard-resume.sh: Same changeTesting
These tests run as
onboard-repair-e2eandonboard-resume-e2ein the nightly pipeline. Requesting a nightly dispatch to validate.Related
Summary by CodeRabbit