perf(policy): batch onboarding preset application#3502
Merged
Conversation
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 (1)
📝 WalkthroughWalkthroughAdds a batch preset application API ( ChangesBatch Preset Application
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
9320-9341: Run the full onboarding E2E matrix before merge for this core-flow change.This modifies preset reconciliation in the core onboarding path; run the recommended onboarding E2E jobs to catch regressions in sandbox lifecycle and provider/channel flows.
As per coding guidelines:
src/lib/onboard.ts“contains core onboarding logic” and changes here affect full sandbox creation/configuration flow, with the listed E2E recommendation set.🤖 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 `@src/lib/onboard.ts` around lines 9320 - 9341, This change touches core onboarding preset reconciliation (applyPresets, applyPreset, waitForPolicyMutation, newlySelected, accessByName in src/lib/onboard.ts); run the full onboarding E2E matrix (all sandbox lifecycle/provider/channel/onboarding permutations) before merging to catch regressions—execute the project's recommended onboarding E2E jobs (including sandbox creation, preset application, provider/channel flows, and cleanup) against this branch and report any failures, re-run locally if flaky, and only merge after green runs and any failures are fixed.
🤖 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 `@src/lib/onboard.ts`:
- Around line 9320-9335: syncPresetSelection has grown with batching logic;
extract the split-and-apply orchestration into a new helper in the policy module
and keep syncPresetSelection as a thin delegator. Create a function (e.g.,
orchestratePresetApplications or applyPresetsBatch) in the policy module that
takes sandboxName and newlySelected, computes builtInPresetNames from
policies.listPresets(), splits newlySelected into builtInNewlySelected and
remainingNewlySelected, and calls waitForPolicyMutation + policies.applyPresets
for the batch and waitForPolicyMutation + policies.applyPreset for each
remaining item; then replace the inlined block in syncPresetSelection with a
single call to the new policy function.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 9320-9341: This change touches core onboarding preset
reconciliation (applyPresets, applyPreset, waitForPolicyMutation, newlySelected,
accessByName in src/lib/onboard.ts); run the full onboarding E2E matrix (all
sandbox lifecycle/provider/channel/onboarding permutations) before merging to
catch regressions—execute the project's recommended onboarding E2E jobs
(including sandbox creation, preset application, provider/channel flows, and
cleanup) against this branch and report any failures, re-run locally if flaky,
and only merge after green runs and any failures are fixed.
🪄 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: 447da395-9749-491a-84e7-270621c7f897
📒 Files selected for processing (6)
src/lib/onboard.tssrc/lib/policy/index.tstest/install-preflight.test.tstest/onboard-preset-diff.test.tstest/policies.test.tstest/policy-tiers-onboard.test.ts
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryPi Semantic E2E AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25842822871
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
69-71: Run the onboarding E2Es for the extracted preset-sync path.This import reroutes a core onboarding step through the new helper, so it’s worth exercising the recommended onboarding suites before merge, especially
cloud-e2eandsandbox-operations-e2e, plus a messaging flow if preset-driven channels are part of the scenario.As per coding guidelines,
src/lib/onboard.ts: "This file contains core onboarding logic. Changes here affect the full sandbox creation and configuration flow."🤖 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 `@src/lib/onboard.ts` around lines 69 - 71, The new import of syncPresetSelection from ./onboard/policy-preset-sync changes a core onboarding step and must be validated by running the onboarding E2E suites; run the cloud-e2e and sandbox-operations-e2e test suites and exercise any preset-driven messaging flows to ensure syncPresetSelection behaves correctly during full sandbox creation and configuration, verify no regressions in src/lib/onboard.ts onboarding paths, and report/fix any failures in the policy-preset-sync module (syncPresetSelection) before merging.
🤖 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 `@src/lib/onboard/policy-preset-sync.ts`:
- Around line 54-67: The current logic batches all built-in presets up front
which reorders merges relative to the original sequential application; change it
so batching only happens when every newlySelected preset is built-in, otherwise
preserve original target order by iterating newlySelected and applying each
preset in sequence. Concretely, in the accessByName === false branch use
policies.listPresets() to compute builtInPresetNames as now, then if
builtInNewlySelected.length>0 && remainingNewlySelected.length===0 call
waitForPolicyMutation with policies.applyPresets(sandboxName,
builtInNewlySelected); otherwise loop over newlySelected and for each name call
waitForPolicyMutation(`applyPreset(${name})`, () =>
policies.applyPreset(sandboxName, name)) so built-in and custom presets are
applied in the original order.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 69-71: The new import of syncPresetSelection from
./onboard/policy-preset-sync changes a core onboarding step and must be
validated by running the onboarding E2E suites; run the cloud-e2e and
sandbox-operations-e2e test suites and exercise any preset-driven messaging
flows to ensure syncPresetSelection behaves correctly during full sandbox
creation and configuration, verify no regressions in src/lib/onboard.ts
onboarding paths, and report/fix any failures in the policy-preset-sync module
(syncPresetSelection) before merging.
🪄 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: 6b28286a-ad4b-4f48-829f-77b7ded1b636
📒 Files selected for processing (2)
src/lib/onboard.tssrc/lib/onboard/policy-preset-sync.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25843302449
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25844327975
|
…s' into perf/batch-onboard-policy-presets
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 25846282461
|
jyaunches
approved these changes
May 14, 2026
12 tasks
miyoungc
added a commit
that referenced
this pull request
May 14, 2026
## Summary Refreshes the NemoClaw documentation for the local `main` changes included in the 0.0.42 release. The update adds release notes, updates the affected user-facing setup and troubleshooting pages, bumps docs metadata to 0.0.42, and regenerates the matching user skills. ## Changes - #3537 -> `docs/reference/commands.md`, `docs/reference/troubleshooting.md`: Documented host-level status fields, cloudflared state-specific recovery hints, and Local Ollama auth proxy status diagnostics. - #3454 -> `docs/get-started/prerequisites.md`, `docs/get-started/quickstart.md`: Documented macOS Docker-driver onboarding and removed the expectation that standard macOS setup needs a VM driver helper. - #3514 -> `docs/inference/use-local-inference.md`: Documented compatible-endpoint retry behavior for reasoning-only smoke responses. - #3448 -> `docs/reference/commands.md`, `docs/manage-sandboxes/messaging-channels.md`: Documented canonical channel names and policy preset hints after `channels add`. - #3520 -> `docs/about/release-notes.md`: Captured clearer GPU recovery and uninstall wording in the 0.0.42 release notes. - #3313 -> `docs/get-started/quickstart.md`, `docs/reference/troubleshooting.md`: Documented stronger dashboard port detection and rollback when a forward cannot start. - #3502 -> `docs/about/release-notes.md`: Captured batched onboarding policy preset application in the 0.0.42 release notes. - #3505 -> `docs/reference/troubleshooting.md`: Documented the top-level Colima socket path. - #3421 -> `docs/about/release-notes.md`: Captured idempotent installer shim logging in the 0.0.42 release notes. - Updated `docs/project.json`, `docs/versions1.json`, and regenerated `.agents/skills/nemoclaw-user-*` outputs. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] 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) --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - v0.0.42 * **Documentation** * Enhanced macOS onboarding guidance for Docker gateway setup * Improved dashboard port conflict handling with automatic rollback * Better local Ollama inference diagnostics and authentication proxy checks * Clarified status command output and recovery procedures * Refined messaging channel setup documentation * **Chores** * Version bump to 0.0.42 <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3540) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3540){kind=link}
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
Batch built-in policy preset application during onboarding so selected presets are merged in memory and submitted to OpenShell with a single
policy set --wait. This reduces repeated gateway policy waits while preserving final policy and registry state.Changes
applyPresets()insrc/lib/policy/index.tsto merge multiple built-in presets, preserve endpoint disclosure logs, submit one policy update, and record applied preset names.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit