refactor(onboard): move policy setup into policy-selection - #8407
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
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:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesOnboarding policy operations now use a centralized application factory. The onboarding flow removes duplicated policy helpers and direct imports. Tests verify mutation locking and setup sequencing. The architecture budget lowers the allowed fan-out from 219 to 212. ChangesOnboarding policy application
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit ac76ff4 in the TypeScript / code-coverage/cliThe overall coverage in commit ac76ff4 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
4 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-selection-application.test.ts`:
- Around line 12-14: Update the withSandboxMutationLock mock in the test to
accept the production callback signature and execute the supplied callback
instead of returning lockResult directly. Remove the unknown cast around the
mock, then assert the callback’s setup result or another observable policy
effect while retaining the sandbox-name assertion as a secondary check.
🪄 Autofix
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: 4228f56e-2601-4f48-bc64-930d7f691c14
📒 Files selected for processing (1)
src/lib/onboard/policy-selection-application.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Moves onboarding policy setup from
src/lib/onboard.tsintosrc/lib/onboard/policy-selection.ts.This keeps
onboard.tsresponsible for sequencing and reduces its fan-out budget from 219 to 212 without changing onboarding behavior.Related Issue
Part of #7695
Changes
createOnboardPolicyApplicationto group the policy setup dependencies used bysrc/lib/onboard.ts.policy-selection.ts.src/lib/onboard.tsfan-out budget from 219 to 212.Type of Change
Quality Gates
a2a4354e6e6622ddbd5ca6c5581a0cba4477e8e5against base SHA2f297843b8fddad80b046eaf34f2987bdd922711.Documentation Writer Review
no-docs-neededci/source-architecture-budget.json,src/lib/onboard.ts,src/lib/onboard/policy-selection.ts, andsrc/lib/onboard/policy-selection-application.test.tsfor PR commita2a4354e6e6622ddbd5ca6c5581a0cba4477e8e5against base SHA2f297843b8fddad80b046eaf34f2987bdd922711. The current PR commit preserves the Personal policy tier added onmainwhile moving existing policy application wiring and adding a regression test for the sandbox mutation lock. It does not change a user-visible API, CLI, configuration, workflow, default, output, or error. The review covered terminology, structure, voice, test-title wording, and code-sample presentation; no documentation or code sample changed.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shdoes not change.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHub — all 13 commits appear asVerified.pre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable — normal hooks passed for the contributor-authored commits. GitHub automation created the current conflict-resolution merge commit; its commit-lint, build-and-typecheck, and static checks passed.cli-test-shards1–8 andcli-testspassed for PR commita2a4354e6e6622ddbd5ca6c5581a0cba4477e8e5. No standalone local test suite ran after the merge.E2E / PR Gatehas not produced a result for the current PR commit.npm run docsbuilds without warnings (doc changes only) — not applicable; no documentation changes.Sensitive-Path Security Review
Independent review found no security finding for PR commit
a2a4354e6e6622ddbd5ca6c5581a0cba4477e8e5against base SHA2f297843b8fddad80b046eaf34f2987bdd922711.Independent maintainer approval remains a separate merge requirement.
GitHub Actions
GitHub Actions is authoritative for PR commit
a2a4354e6e6622ddbd5ca6c5581a0cba4477e8e5against base SHA2f297843b8fddad80b046eaf34f2987bdd922711.Source CI passed.
E2E / PR Gatehas not produced a result for the current PR commit.No check waiver is recorded.
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit