feat(policy): add policy-remove command to remove applied presets#1822
Merged
cv merged 6 commits intoNVIDIA:mainfrom Apr 15, 2026
Merged
feat(policy): add policy-remove command to remove applied presets#1822cv merged 6 commits intoNVIDIA:mainfrom
cv merged 6 commits intoNVIDIA:mainfrom
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughOnboard now revokes deselected policy presets with retry handling; new policy-removal primitives modify YAML at the Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as "nemoclaw (policy-remove)"
participant SelectUI as "selectForRemoval"
participant Policies as "policies (removePreset / removePresetFromPolicy)"
participant Sandbox as "Sandbox (policy store)"
participant Registry as "Sandbox Registry"
User->>CLI: run "nemoclaw <sandbox> policy-remove [--dry-run]"
CLI->>Policies: list presets, compute appliedPresets
CLI->>SelectUI: selectForRemoval(appliedPresets)
SelectUI->>User: prompt choose preset
User-->>SelectUI: choose preset / cancel
SelectUI-->>CLI: return presetName or null
CLI->>Policies: load presetEntries (preview endpoints)
CLI->>User: show preview
alt dry-run
CLI->>User: announce no changes and exit
else
CLI->>User: prompt confirmation
User-->>CLI: confirm
CLI->>Policies: removePreset(sandboxName, presetName)
Policies->>Sandbox: fetch current policy YAML
Policies->>Policies: removePresetFromPolicy(currentYAML, presetEntries)
Policies->>Sandbox: apply updated YAML (policy-set via temp file)
Policies->>Registry: remove presetName from sandbox.policies
Policies-->>CLI: return success/failure
CLI->>User: final status
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard.ts`:
- Around line 4147-4160: The retry loop over deselected presets currently only
retries on thrown errors from policies.removePreset(sandboxName, name) but
ignores a false return; update the loop so that after calling
removePreset(sandboxName, name) you treat a falsey return as a failure (throw a
new Error or re-use a descriptive error) to trigger the retry logic, preserving
the existing handling for the specific "sandbox not found" message and the
3-attempt/backoff behavior; reference the variables and functions involved:
deselected, sandboxName, policies.removePreset, and the attempt/retry block so
the false return path does not silently mark a preset as removed.
In `@src/lib/policies.ts`:
- Around line 311-342: remove reporting and registry updates when
removePresetFromPolicy made no changes: after calling parseCurrentPolicy and
removePresetFromPolicy (symbols: parseCurrentPolicy, removePresetFromPolicy),
compare the returned updated string to the original currentPolicy; if they are
identical, skip creating the temp file, avoid calling
run(buildPolicySetCommand(...)) and console.log("Removed preset..."), and do not
modify the sandbox via registry.updateSandbox — instead log/return early
indicating nothing changed. This ensures we only write, apply, and unregister
the preset when at least one policy entry was actually removed.
In `@src/nemoclaw.ts`:
- Around line 1256-1259: The call to policies.removePreset(sandboxName, answer)
ignores its boolean return so failures are treated as success; update the code
that follows the confirmation prompt to check the return value of
removePreset(sandboxName, answer) and, if it returns false, propagate the
failure by exiting with a non-zero status (e.g., process.exit(1) or return a
non-zero command result) and log an error message; reference the confirm logic
around askPrompt(...) and the policies.removePreset(...) invocation and ensure
the failure path does not fall through to a successful exit.
🪄 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: Pro
Run ID: 53aa80f1-41f0-4c67-9701-417e500e6ab4
📒 Files selected for processing (4)
src/lib/onboard.tssrc/lib/policies.tssrc/nemoclaw.tstest/policies.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard.ts (1)
4138-4162:⚠️ Potential issue | 🟠 MajorResume can preserve a preset the user already deselected.
onSelection(interactiveChoice)persists the new target set before the removal loop succeeds. IfremovePreset()exhausts retries here, the session now records the preset as deselected even though it is still live. On resume, theselectedPresetsfast-path only reapplies missing presets and never retries that removal, so the sandbox can stay permanently out of sync with the saved selection. Please make the resume path reconcile removals too, or only persist the new selection after both loops complete successfully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 4138 - 4162, The bug is that onSelection(interactiveChoice) persists the new selection before removals complete, so failed removePreset(sandboxName, name) leaves the session out of sync; fix by deferring persistence until after both the deselection loop (which calls policies.removePreset) and the addition loop complete successfully — move the onSelection(interactiveChoice) call so it runs only after the removal retries and any addition logic finish, or alternatively enhance the resume/selectedPresets fast-path to reconcile removals by checking applied vs interactiveChoice and invoking policies.removePreset for any names that should be removed; target functions/variables: onSelection, interactiveChoice, applied, policies.removePreset, selectedPresets fast-path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/policies.ts`:
- Around line 311-317: Currently we call parseCurrentPolicy(rawPolicy) and then
removePresetFromPolicy(...) which can convert a failed parse (parseCurrentPolicy
returning an empty string) into a fresh policy skeleton and proceed to write it;
instead detect parse failure and fail closed: after computing currentPolicy (the
return value of parseCurrentPolicy), if currentPolicy is an empty string or
otherwise indicates a parse error, log an error mentioning presetName and the
read failure and return false immediately (do not call removePresetFromPolicy);
keep the existing updated === currentPolicy equality check only for legitimate
parsed policies.
---
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 4138-4162: The bug is that onSelection(interactiveChoice) persists
the new selection before removals complete, so failed removePreset(sandboxName,
name) leaves the session out of sync; fix by deferring persistence until after
both the deselection loop (which calls policies.removePreset) and the addition
loop complete successfully — move the onSelection(interactiveChoice) call so it
runs only after the removal retries and any addition logic finish, or
alternatively enhance the resume/selectedPresets fast-path to reconcile removals
by checking applied vs interactiveChoice and invoking policies.removePreset for
any names that should be removed; target functions/variables: onSelection,
interactiveChoice, applied, policies.removePreset, selectedPresets fast-path.
🪄 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: Pro
Run ID: 92377f6e-0c78-435b-8c13-320412d3f623
📒 Files selected for processing (4)
src/lib/onboard.tssrc/lib/policies.tssrc/nemoclaw.tstest/policies.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/nemoclaw.ts
- test/policies.test.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/policies.ts (1)
272-277:⚠️ Potential issue | 🟠 MajorDetect semantic no-op before serializing policy.
removePresetFromPolicy()re-serializes even when no preset keys actually exist. That can change formatting only, so Line 319 no-op detection may be bypassed and still report success/update registry without real removal.💡 Proposed fix
- for (const key of presetKeys) { - delete existingNp[key]; - } - - current.network_policies = existingNp; - return YAML.stringify(current); + let removed = false; + for (const key of presetKeys) { + if (Object.prototype.hasOwnProperty.call(existingNp, key)) { + delete existingNp[key]; + removed = true; + } + } + + if (!removed) return normalizedCurrentPolicy; + + current.network_policies = existingNp; + return YAML.stringify(current);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/policies.ts` around lines 272 - 277, In removePresetFromPolicy(), avoid re-serializing when nothing is changed: before calling YAML.stringify(current), check whether any of the presetKeys were present/deleted from existingNp (e.g., track a deleted flag while iterating or compute intersection of presetKeys and Object.keys(existingNp)); if no keys were removed, return the original YAML string (or a no-op indicator) instead of re-serializing current to prevent formatting-only changes; references: presetKeys, existingNp, current, YAML.stringify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/policies.ts`:
- Around line 272-277: In removePresetFromPolicy(), avoid re-serializing when
nothing is changed: before calling YAML.stringify(current), check whether any of
the presetKeys were present/deleted from existingNp (e.g., track a deleted flag
while iterating or compute intersection of presetKeys and
Object.keys(existingNp)); if no keys were removed, return the original YAML
string (or a no-op indicator) instead of re-serializing current to prevent
formatting-only changes; references: presetKeys, existingNp, current,
YAML.stringify.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18757cba-7a94-4363-81af-68be2cf17a20
📒 Files selected for processing (1)
src/lib/policies.ts
wscurran
added
enhancement: feature
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
4214-4246: Extract shared retry logic for preset mutations to reduce branching.The remove/apply loops now duplicate the same retry/error pattern. A small local helper will keep this path easier to maintain and reduce complexity drift.
♻️ Proposed refactor
+ const mutatePresetWithRetry = (action, name, fn) => { + for (let attempt = 0; attempt < 3; attempt += 1) { + try { + if (!fn()) { + throw new Error(`Failed to ${action} preset '${name}'.`); + } + return; + } catch (err) { + const message = err && err.message ? err.message : String(err); + if (!message.includes("sandbox not found") || attempt === 2) { + throw err; + } + sleep(2); + } + } + }; + for (const name of deselected) { - for (let attempt = 0; attempt < 3; attempt += 1) { - try { - if (!policies.removePreset(sandboxName, name)) { - throw new Error(`Failed to remove preset '${name}'.`); - } - break; - } catch (err) { - const message = err && err.message ? err.message : String(err); - if (!message.includes("sandbox not found") || attempt === 2) { - throw err; - } - sleep(2); - } - } + mutatePresetWithRetry("remove", name, () => policies.removePreset(sandboxName, name)); } for (const name of newlySelected) { - for (let attempt = 0; attempt < 3; attempt += 1) { - try { - if (!policies.applyPreset(sandboxName, name)) { - throw new Error(`Failed to apply preset '${name}'.`); - } - break; - } catch (err) { - const message = err && err.message ? err.message : String(err); - if (!message.includes("sandbox not found") || attempt === 2) { - throw err; - } - sleep(2); - } - } + mutatePresetWithRetry("apply", name, () => policies.applyPreset(sandboxName, name)); }As per coding guidelines, "Limit cyclomatic complexity to 20 in JavaScript/TypeScript files, with target of 15".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 4214 - 4246, Extract the duplicated retry pattern into a small local helper (e.g., retryPresetMutation) that accepts an async mutation function and preset name and handles the 3-attempt loop, error message inspection for "sandbox not found", sleep(2) backoff, and final throw; then replace both loops that call policies.removePreset(sandboxName, name) and policies.applyPreset(sandboxName, name) (iterating over deselected and newlySelected) with calls to this helper, preserving the same error messages (e.g., "Failed to remove preset '...'" / "Failed to apply preset '...'") and using sandboxName and sleep as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 4214-4246: Extract the duplicated retry pattern into a small local
helper (e.g., retryPresetMutation) that accepts an async mutation function and
preset name and handles the 3-attempt loop, error message inspection for
"sandbox not found", sleep(2) backoff, and final throw; then replace both loops
that call policies.removePreset(sandboxName, name) and
policies.applyPreset(sandboxName, name) (iterating over deselected and
newlySelected) with calls to this helper, preserving the same error messages
(e.g., "Failed to remove preset '...'" / "Failed to apply preset '...'") and
using sandboxName and sleep as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d7da51dc-bb80-45e9-a77e-d2264ba780f4
📒 Files selected for processing (2)
src/lib/onboard.tssrc/nemoclaw.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/nemoclaw.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/policies.ts (1)
273-278:⚠️ Potential issue | 🟠 MajorReturn the original YAML when nothing was actually deleted.
If none of
presetKeysexist innetwork_policies, this still reserializes the document. That can makeupdated !== currentPolicyon formatting alone, soremovePreset()may callpolicy setand drop the preset from the registry even though the live policy rules were unchanged. A regression case where the preset key is already absent would be worth adding too.🛠️ Proposed fix
- for (const key of presetKeys) { - delete existingNp[key]; - } + let removed = false; + for (const key of presetKeys) { + if (Object.prototype.hasOwnProperty.call(existingNp, key)) { + delete existingNp[key]; + removed = true; + } + } + + if (!removed) return normalizedCurrentPolicy; current.network_policies = existingNp; return YAML.stringify(current);Also applies to: 320-323
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/policies.ts` around lines 273 - 278, The code always reserializes and returns YAML.stringify(current) even when none of the keys in presetKeys were present, causing no-op changes; update the removePreset logic to detect whether any deletion actually occurred (track a boolean like "deleted" while iterating presetKeys against existingNp) and only overwrite current.network_policies / return a new YAML when at least one key was removed; if no keys were removed, return the original YAML string (or originalPolicy variable) unchanged. Apply the same change to the analogous block around the other occurrence (the block at the indicated second location).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/policies.ts`:
- Around line 273-278: The code always reserializes and returns
YAML.stringify(current) even when none of the keys in presetKeys were present,
causing no-op changes; update the removePreset logic to detect whether any
deletion actually occurred (track a boolean like "deleted" while iterating
presetKeys against existingNp) and only overwrite current.network_policies /
return a new YAML when at least one key was removed; if no keys were removed,
return the original YAML string (or originalPolicy variable) unchanged. Apply
the same change to the analogous block around the other occurrence (the block at
the indicated second location).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e08388d-025a-4b05-ad07-e38525e069e1
📒 Files selected for processing (4)
src/lib/onboard.tssrc/lib/policies.tssrc/nemoclaw.tstest/policies.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/nemoclaw.ts
cv
approved these changes
Apr 15, 2026
3 tasks
cv
pushed a commit
that referenced
this pull request
Apr 15, 2026
## Summary - Document `nemoclaw <name> snapshot create/list/restore` commands (from #1892) - Document `nemoclaw <name> policy-remove` command (from #1822) - Document `nemoclaw <name> rebuild` command with version staleness detection (from #1870) - Document `nemoclaw backup-all` command (from #1870) - Update `status` to mention live enforced policy display (from #1896) - Update `connect` and `status` to mention version staleness warnings (from #1870) - Update `destroy` to reference snapshots and rebuild as alternatives - Add snapshot commands section to backup-restore page - Add `policy-remove` to customize-network-policy page - Add "Sandbox is running an outdated agent version" troubleshooting entry - Bump doc version switcher through 0.0.16 - Regenerate agent skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit hooks pass - [ ] Verify rendered pages in docs build output 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added documentation for policy preset removal command with selection flow * Documented new rebuild command for sandbox upgrades while preserving workspace state * Introduced snapshot commands for creating, listing, and restoring workspace backups * Added bulk backup capability for multiple sandboxes * Enhanced agent version warnings in connect and status commands with remediation guidance * Expanded troubleshooting guide with outdated agent version resolution steps <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Add
nemoclaw <name> policy-removeto remove previously applied network policy presets from a sandbox.node bin/nemoclaw.js my-assistant policy-remove Applied presets: 1) npm — npm and Yarn registry access 2) pypi — Python Package Index (PyPI) access Choose preset to remove: 2 Endpoints that would be removed: pypi.org, files.pythonhosted.org Remove 'pypi' from sandbox 'my-assistant'? [Y/n]: Narrowing sandbox egress — removing: pypi.org, files.pythonhosted.org ✓ Policy version 5 submitted (hash: 440e75a56b48) ✓ Policy version 5 loaded (active version: 5) Removed preset: pypiAlso fix onboard TUI checkbox so unchecking an applied preset actually removes it.
[8/8] Policy presets ────────────────────────────────────────────────── Available policy presets: [ ] brave — Brave Search API access [ ] brew — Homebrew (Linuxbrew) package manager access [ ] discord — Discord API, gateway, and CDN access [ ] github — GitHub.com and GitHub API access (git, gh) [ ] huggingface — Hugging Face Hub, LFS, and Inference API access [ ] jira — Jira and Atlassian Cloud access [✓] npm — npm and Yarn registry access [ ] outlook — Microsoft Outlook and Graph API access > [ ] pypi — Python Package Index (PyPI) access <--- uncheck pypi [ ] slack — Slack API, Socket Mode, and webhooks access [ ] telegram — Telegram Bot API access ↑/↓ j/k move Space toggle a all/none Enter confirm Narrowing sandbox egress — removing: pypi.org, files.pythonhosted.org ✓ Policy version 7 submitted (hash: 440e75a56b48) ✓ Policy version 7 loaded (active version: 7) Removed preset: pypiRelated Issue
Fixes #1727
Changes
Changes
nemoclaw <name> policy-removecommand that reads the sandbox's current policy, strips the selected preset'snetwork_policiesentries, and writes it back viaopenshell policy set--dry-runto preview which endpoints would be narrowedUnimplementedgRPC error handlers that became unreachable after minimum OpenShell version was raised to 0.0.24policy-removeType of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
nemoclaw-contributor-update-docsagent skill to draft changes while complying with the style guide. For example, prompt your agent with "/nemoclaw-contributor-update-docscatch up the docs for the new changes I made in this PR."Signed-off-by: Seven Cheng sevenc@nvidia.com
Summary by CodeRabbit
New Features
Improvements
Tests