fix(onboard): preserve effective policy preset selection across re-onboard

[yimoj]

Summary

Re-onboarding a sandbox could silently reapply Balanced tier defaults and lose the operator's custom preset removals (e.g. a removed npm). This persists the effective policy preset selection to the registry and seeds re-onboard sessions from it, so removals survive both reuse and recreate.

Related Issue

Fixes #4621

Changes

• Add src/lib/onboard/policy-preset-persistence.ts: persists the finalized selection (built-in names only — custom presets stay in customPolicies) and seeds reused/recreated sessions from the recorded applied set.
• Reuse path: seed the fresh session from the full applied set (built-ins plus custom-preset names) so custom presets are not diffed away as "deselected"; carry an intentionally-empty Restricted-tier selection as [] instead of falling back to Balanced. Env overrides (NEMOCLAW_POLICY_PRESETS / NEMOCLAW_POLICY_MODE) still win.
• Add a policyPresetsFinalized registry marker, set only by the post-policy write (never at registration), so boot-time presets from an interrupted run and snapshot clones are not mistaken for a final selection.
• Persist before marking the policies step complete; skip persistence on the policy-skip path (NEMOCLAW_POLICY_MODE=skip/none/no) and on the resume/already-applied branch (which cannot verify the live set exactly).
• Recreate path honors a finalized empty selection, unless custom presets were recorded (their content is discarded on recreate, so fall back to prompting).
• src/lib/onboard.ts change is net-neutral (logic extracted into the new module under src/lib/onboard/).

Type of Change

• Code change (feature, bug fix, or refactor)

Verification

• npm test passes (only pre-existing, environment-specific permission/ownership failures in config-sync, nemoclaw-start, cli, ssrf-parity, migration-state remain — confirmed identical on a clean tree)
• Tests added or updated for new or changed behavior (handler, carry-forward, registry, and the new persistence module — pure decision fns and I/O glue)
• No secrets, API keys, or credentials committed
• npm run typecheck:cli passes; Biome clean; codex review clean

Reproduced end-to-end against the real compiled modules: removing a Balanced default survives re-onboard/reuse, unrelated/custom presets are preserved, Restricted (empty) selections persist, and interrupted/boot-only state correctly falls back to prompting.

---

Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

• New Features

  • Policy preset selections are persisted and correctly carried forward when re-onboarding or recreating a sandbox.
  • Intentionally removed/deselected policies remain removed and won’t be reintroduced.
  • Environment-variable overrides are honored in non-interactive flows and ignored in interactive flows.

• Bug Fixes

  • Registry registration no longer carries a finalized policy marker; finalized state is set only after policy completion.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 48a0963a-c3fb-4e4d-9258-3d2d97d06d6e

📥 Commits

Reviewing files that changed from the base of the PR and between f39db47 and 4e33978.

📒 Files selected for processing (3)

• src/lib/onboard.ts
• src/lib/state/registry.ts
• test/registry.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• test/registry.test.ts
• src/lib/state/registry.ts
• src/lib/onboard.ts

---

📝 Walkthrough

Walkthrough

This PR records operator-applied policy presets as finalized in the sandbox registry, seeds reuse and recreate onboarding flows from that recorded set (preserving intentional empty selections and filtering to built-in presets), and persists the finalized applied set only when live reconciliation occurred.

Changes

Policy Preset Persistence and Carry-Forward

Layer / File(s)	Summary
Registry schema and finalization marker src/lib/state/registry.ts, test/registry.test.ts	SandboxEntry adds optional policyPresetsFinalized; registerSandbox intentionally does not set it; tests verify the marker is set only by later updates.
Persistence module: finalization, resolution, and recording src/lib/onboard/policy-preset-persistence.ts, src/lib/onboard/policy-preset-persistence.test.ts	New module lazily loads builtin presets, implements buildFinalizedPolicyPresetsUpdate, resolveRecreatePolicyPresets, applyRecreatePolicyCarryForward, seedReusedSandboxPolicyPresets, and persistFinalizedPolicyPresets; tests cover builtin filtering, finalized-empty handling, env overrides, recreate seeding, and persistence filtering.
Reuse path: decision and seeding src/lib/onboard/policy-carryforward.ts, src/lib/onboard/policy-carryforward.test.ts, src/lib/onboard/policy-preset-persistence.test.ts	decideReusePolicyPresets determines when to reuse recorded presets vs honor non-interactive env overrides; seedReusedSandboxPolicyPresets applies the recorded set only when prior step was finalized and session has no presets. Tests validate interactive/non-interactive, empty selection preservation, and env precedence.
Handler persistence trigger and flag src/lib/onboard/machine/handlers/policies.ts, src/lib/onboard/machine/handlers/policies.test.ts	Policies handler adds persistAppliedPolicyPresets dependency and reflectsLiveAppliedSet flag; onSelection sets the flag and persistence runs only when reconciliation occurred. Tests add mocks and regression coverage for skip/resume/persist behavior.
Main flow orchestration src/lib/onboard.ts	Replaces old carry-forward import with the new policyPresetCarry module, seeds reused sessions via seedReusedSandboxPolicyPresets, applies recreate carry-forward via applyRecreatePolicyCarryForward, and wires persistFinalizedPolicyPresets into policy handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

fix, onboarding

Suggested reviewers

• cv
• prekshivyas
• ericksoa

Poem

🐰 I hopped along the onboard trail,
Saved your presets without fail,
No defaults sneaking back on roam,
Your sandbox keeps its perfect home,
Carried forward, safe and hale.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: preserving effective policy preset selection across re-onboarding, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR implementation directly addresses all coding objectives from #4621: preserving finalized policy presets via new persistence module, seeding reused/recreated sessions from recorded selections, preventing silent drift to tier defaults, handling empty selections as intentional records, respecting environment overrides, and adding comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to #4621 objectives: policy preset persistence logic, registry marker for finalization state, refactoring onboard.ts to use new persistence module, test coverage for new functionality, and handler updates to persist selections. No unrelated changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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.ts`:
- Around line 3081-3082: The current call
policyPresetCarry.seedReusedSandboxPolicyPresets(sandboxName,
isNonInteractive()) is seeding reuse state too early; move this call out of the
pre-decision path and only invoke it after reuse is actually confirmed (i.e.,
the code path that commits to reusing the existing session), so it is not
executed on provider/model-drift or when the user declines reuse. Locate where
the reuse decision is finalized (the function or branch that handles confirmed
reuse) and call seedReusedSandboxPolicyPresets there using the same sandboxName
and isNonInteractive() arguments; remove or guard the original pre-decision
invocation to prevent leaking reuse semantics into recreate/abort flows.

🪄 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: cfdb74b7-2825-49b6-b3a1-b972f49b1d8f

📥 Commits

Reviewing files that changed from the base of the PR and between c8df6ae and 1aa0cd0.

📒 Files selected for processing (9)

• src/lib/onboard.ts
• src/lib/onboard/machine/handlers/policies.test.ts
• src/lib/onboard/machine/handlers/policies.ts
• src/lib/onboard/policy-carryforward.test.ts
• src/lib/onboard/policy-carryforward.ts
• src/lib/onboard/policy-preset-persistence.test.ts
• src/lib/onboard/policy-preset-persistence.ts
• src/lib/state/registry.ts
• test/registry.test.ts

[prekshivyas]

APPROVE.

Traced all onboard entry points — the effective preset is preserved on reuse, recreate, and re-onboard. Reuse seeding (onboard.ts:3104/3165) routes the recorded applied set through setupPoliciesWithSelection's selectedPresets resume branch (policy-selection.ts:319-329), bypassing the tier selector at :417-422 that was re-adding npm — the exact root cause from the issue thread. Persist is gated on reflectsLiveAppliedSet and runs before recordStepComplete (policies.ts:226-236), so skip/resume paths don't clobber the live set. policyPresetsFinalized is set only post-policy (registry.ts:216), so clones/interrupted runs aren't mistaken for a final selection.

Security: no path silently widens egress — buildFinalizedPolicyPresetsUpdate filters to built-in names and excludes custom-name collisions; an empty Restricted selection persists as []; recreate-with-customs degrades to a prompt rather than dropping silently (documented, tested).

Non-blocking: the seed call sits in both confirmed-reuse branches and seedReusedSandboxPolicyPresets self-guards on the finalized marker + in-progress --resume, so it's idempotent — no change needed. Resolves #4621.

Signed-off-by: Prekshi Vyas prekshiv@nvidia.com