fix(onboard): fall back to tier suggestions on bad NEMOCLAW_POLICY_MODE - #2434
Merged
Conversation
Step 8/8 of onboard runs after the sandbox is already created. An unrecognised NEMOCLAW_POLICY_MODE value used to trigger process.exit(1) there, which left the caller with a built sandbox and zero policy presets applied — not even the balanced-tier defaults. The reporter hit this by typing NEMOCLAW_POLICY_MODE=restricted (a valid tier name, not a valid mode) and was told to hand-fix from the outside. Warn instead of exiting, suggest NEMOCLAW_POLICY_TIER=<value> when the value happens to be a known tier name, and fall through to the tier-derived suggestions so the sandbox stays usable. Resolves #2429. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
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)
📝 WalkthroughWalkthroughWhen 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)
Comment |
ericksoa
approved these changes
Apr 24, 2026
ericksoa
left a comment
ericksoa
left a comment
Contributor
There was a problem hiding this comment.
Clean fix. Converting the hard exit to a graceful fallback is the right call — step 8/8 runs after the sandbox is built, so exiting leaves the user worse off than falling through to tier-derived defaults.
The tiers.getTier(policyMode) hint for the common tier-name-in-mode-variable mistake is a nice touch. Both test cases (tier-name hit and non-tier garbage) cover the branching well.
LGTM.
This was referenced Apr 27, 2026
cv
pushed a commit
that referenced
this pull request
Apr 29, 2026
…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 -->
DemianHeyGen
pushed a commit
to DemianHeyGen/NemoClaw
that referenced
this pull request
Apr 30, 2026
…DE (NVIDIA#2434) <!-- markdownlint-disable MD041 --> ## Summary Step 8/8 of onboard runs after the sandbox is already created. An unrecognised NEMOCLAW_POLICY_MODE value used to trigger process.exit(1) there, which left the caller with a built sandbox and zero policy presets applied — not even the balanced-tier defaults. The reporter hit this by typing NEMOCLAW_POLICY_MODE=restricted (a valid tier name, not a valid mode) and was told to hand-fix from the outside. ## Related Issue Resolves NVIDIA#2429 ## Changes Warn instead of exiting, suggest NEMOCLAW_POLICY_TIER=<value> when the value happens to be a known tier name, and fall through to the tier-derived suggestions so the sandbox stays usable. ## Type of Change - [X] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. --> - [X] `npx prek run --all-files` passes - [X] `npm test` passes - [X] Tests added or updated for new or changed behavior - [X] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure <!-- If an AI agent authored or co-authored this PR, check the box and name the tool. Remove this section for fully human-authored PRs. --> - [X] AI-assisted — tool: Claude Code<!-- e.g., Claude Code, Cursor, GitHub Copilot --> --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Onboarding no longer terminates when an unsupported policy mode value is provided. The system displays warnings with suggestions for valid alternatives and continues setup with appropriate presets. When the invalid value matches an existing policy tier, a helpful environment variable suggestion is included. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
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 -->
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
Step 8/8 of onboard runs after the sandbox is already created. An unrecognised NEMOCLAW_POLICY_MODE value used to trigger process.exit(1) there, which left the caller with a built sandbox and zero policy presets applied — not even the balanced-tier defaults. The reporter hit this by typing NEMOCLAW_POLICY_MODE=restricted (a valid tier name, not a valid mode) and was told to hand-fix from the outside.
Related Issue
Resolves #2429
Changes
Warn instead of exiting, suggest NEMOCLAW_POLICY_TIER= when the value happens to be a known tier name, and fall through to the tier-derived suggestions so the sandbox stays usable.
Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit