fix(policy): remove a preset the gateway enforces without a local record - #9306
Conversation
policy list reports a preset as active when either the registry or the gateway holds it, but policy remove consulted the registry alone. The one state list exists to flag, active on gateway missing from local state, was therefore the one state with no removal path, even though removePreset already reconciles it without a registry entry. policy remove now builds a single removable set from both sources and uses it for the named-preset guard and the interactive picker alike. A gateway that cannot be queried is not evidence of absence, so the command falls back to local state and says which evidence it had. Applying a built-in preset to a sandbox that is missing from the registry no longer returns success in silence, closing one way the divergence is created. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 7ebf1a6 in the TypeScript / code-coverage/cliThe overall coverage in commit 7ebf1a6 in the Show a code coverage summary of the most impacted files.
Updated |
|
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 (8)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change reconciles gateway and local policy state. Built-in presets can remain active on the gateway when local recording fails. Policy removal now supports gateway-only presets and unavailable gateway queries. ChangesPolicy state reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to This PR changes policy enforcement and local/gateway reconciliation behavior, with targeted validation reported clean. However, the required sensitive-path review or maintainer waiver is not recorded, so the PR is not merge-ready until that approval is completed. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-9306.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology 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 for the commit under review. Recommended E2E: Manual-only E2E: 2 optional E2E recommendations
1 warning · 0 suggestionsWarningsWarnings do not block.
|
prekshivyas
left a comment
There was a problem hiding this comment.
Policy removal now uses the union of locally recorded and live-gateway presets while preserving the gateway-unreachable distinction. The built-in/custom persistence asymmetry is explicit, the mutation remains fail-closed for unknown or absent presets, and user-facing documentation covers the new recovery paths.
Cross-issue sweep: no additional candidate issues found.
Security review: secrets/credentials — PASS; input validation/sanitization — PASS; authentication/authorization — PASS; dependencies — PASS; error handling/logging — PASS; cryptography/data protection — PASS; configuration/security headers — PASS; security testing — PASS; system security — PASS.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry required before planning the v0.0.110 release. The entry summarizes user-facing changes merged since v0.0.109 and links each change to its published documentation route and source PR. ## Changes - Add `docs/changelog/2026-08-17.mdx` with the exact `## v0.0.110` release heading. - Cover managed local inference, endpoint validation, onboarding and recovery, explicit experimental Portable OpenClaw, messaging and policy cleanup, backup and security hardening, and release qualification. - Preserve the documentation skip list and the current supported-agent matrix; test-only refactors, dormant activation work, and Pi-only changes are intentionally excluded. ### Source-to-doc mapping - #8711 -> `docs/changelog/2026-08-17.mdx`: Add the Muse Glimmer llama.cpp profile. - #9099 -> `docs/changelog/2026-08-17.mdx`: Update the Muse Glimmer vLLM runtime. - #9319 -> `docs/changelog/2026-08-17.mdx`: Select the provider required by an explicit serving profile. - #9311 -> `docs/changelog/2026-08-17.mdx`: Report probe-image pull failures separately. - #9345 -> `docs/changelog/2026-08-17.mdx`: Reuse mirrored Windows Ollama. - #9284 -> `docs/changelog/2026-08-17.mdx`: Complete the required Ollama upgrade. - #9320 -> `docs/changelog/2026-08-17.mdx`: Reject unsafe custom endpoint URLs before mutation. - #9119 -> `docs/changelog/2026-08-17.mdx`: Reject unsupported custom endpoint URL components. - #9236 -> `docs/changelog/2026-08-17.mdx`: Require native Anthropic tool-use evidence. - #9347 -> `docs/changelog/2026-08-17.mdx`: Distinguish Gemini runtime 404 diagnostics. - #9307 -> `docs/changelog/2026-08-17.mdx`: Preserve the recorded API family when only the model drifts. - #9233 -> `docs/changelog/2026-08-17.mdx`: Fail incomplete Hermes route synchronization. - #9185 -> `docs/changelog/2026-08-17.mdx`: Serialize Model Router lifecycle work across gateways. - #9112 -> `docs/changelog/2026-08-17.mdx`: Stop Model Router after the last routed sandbox is destroyed. - #9229 -> `docs/changelog/2026-08-17.mdx`: Verify fresh sandbox execution readiness. - #9299 -> `docs/changelog/2026-08-17.mdx`: Verify a separate agent API host forward before reporting ready. - #9318 -> `docs/changelog/2026-08-17.mdx`: Honor explicit sandbox recreation. - #9325 -> `docs/changelog/2026-08-17.mdx`: Measure readiness reuse windows from collection completion. - #9352 -> `docs/changelog/2026-08-17.mdx`: Guide users away from the deprecated global start command. - #9370 -> `docs/changelog/2026-08-17.mdx`: Persist managed OpenClaw agent identity. - #9366 -> `docs/changelog/2026-08-17.mdx`: Pass messaging dependencies during reused onboarding. - #9321 -> `docs/changelog/2026-08-17.mdx`: Detect proxied connect sessions. - #9285 -> `docs/changelog/2026-08-17.mdx`: Run probe-only recovery when absent authority cannot be created. - #9282 -> `docs/changelog/2026-08-17.mdx`: Complete probe-only recovery without platform evidence. - #8920 -> `docs/changelog/2026-08-17.mdx`: Preserve legacy gateway identity. - #9198 -> `docs/changelog/2026-08-17.mdx`: Report sandbox config-read failures. - #9201 -> `docs/changelog/2026-08-17.mdx`: Remove only the exact Docker orphan on destroy. - #9176 -> `docs/changelog/2026-08-17.mdx`: Use rootless Podman for Portable lifecycle operations. - #9197 -> `docs/changelog/2026-08-17.mdx`: Preflight Portable CPU delegation. - #9289 -> `docs/changelog/2026-08-17.mdx`: Narrow Portable policy defaults. - #9270 -> `docs/changelog/2026-08-17.mdx`: Preserve Portable model intent. - #9339 -> `docs/changelog/2026-08-17.mdx`: Reconcile timed-out Portable stop state. - #9209 -> `docs/changelog/2026-08-17.mdx`: Clean receipt-owned Portable Podman resources. - #9186 -> `docs/changelog/2026-08-17.mdx`: Separate Podman activation readiness. - #9376 -> `docs/changelog/2026-08-17.mdx`: Settle Portable OpenClaw pairing before readiness. - #9296 -> `docs/changelog/2026-08-17.mdx`: Retire messaging channel presets the host no longer configures. - #9327 -> `docs/changelog/2026-08-17.mdx`: Drop retired channels from reused messaging selections. - #9306 -> `docs/changelog/2026-08-17.mdx`: Remove gateway-enforced presets without a local record. - #9248 -> `docs/changelog/2026-08-17.mdx`: Activate Google Chat pairing approval. - #9374 -> `docs/changelog/2026-08-17.mdx`: Accept schema-owned messaging plan fields. - #9317 -> `docs/changelog/2026-08-17.mdx`: Accept safe hard-linked package files during backup. - #9288 -> `docs/changelog/2026-08-17.mdx`: Remove managed CLI shims with destroyed user data. - #9239 -> `docs/changelog/2026-08-17.mdx`: Read voice credentials from fixed descriptors. - #9269 -> `docs/changelog/2026-08-17.mdx`: Accept bounded native OpenClaw device modes. - #9371 -> `docs/changelog/2026-08-17.mdx`: Isolate OpenClaw startup-guard output. - #9351 -> `docs/changelog/2026-08-17.mdx`: Restore staging Launchable validation. - #9350 -> `docs/changelog/2026-08-17.mdx`: Retry transient collaborator-permission reads. - #9353 -> `docs/changelog/2026-08-17.mdx`: Retry transient exact-artifact downloads. - #9226 -> `docs/changelog/2026-08-17.mdx`: Add bounded Brev readiness diagnostics. - #9237 -> `docs/changelog/2026-08-17.mdx`: Report same-commit E2E reliability. - #9232 -> `docs/changelog/2026-08-17.mdx`: Execute native-runtime qualification. - #9275 -> `docs/changelog/2026-08-17.mdx`: Define E2E selection and retry guidance. - #9234 -> `docs/changelog/2026-08-17.mdx`: Move documentation review after merge. - #9365 -> `docs/changelog/2026-08-17.mdx`: Mount documentation reviewer inputs before startup. ## 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) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated release-entry contract. - [ ] Tests not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; documentation-only change. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (7 passed) - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to one prose-only changelog page; `npm run docs` passed the repository's strict documentation gate. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with 0 errors and the 2 existing Fern warnings. - [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) — the SPDX header is present; dated changelog pages intentionally do not use frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.110. * Documented experimental managed llama.cpp and Portable OpenClaw profiles. * Covered inference validation, onboarding and recovery improvements, rootless lifecycle handling, messaging and policy updates, backups, credential handling, filesystem protections, and release qualification updates. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
policy listreports a preset as active when either the local registry or the live gateway holds it, butpolicy removeconsulted the registry alone and exited non-zero withPreset 'X' is not applied., so the one statepolicy listexists to flag — active on gateway, missing from local state — was the only state with no removal path.policy removenow builds a single removable set from both sources and uses it for the named-preset guard and the interactive picker alike, and it distinguishes a gateway it could not query from a gateway that does not hold the preset. Applying a built-in preset to a sandbox that is missing from the registry no longer returns success in silence, which closes one way that divergence is created.Related Issue
Fixes #9295
Changes
removeSandboxPolicyUnlockedinsrc/lib/actions/sandbox/policy-channel.tsderives oneremovableset fromgetAppliedPresetsandgetGatewayPresets, and both the named-preset guard and theselectForRemovalpicker read it. Computing it once is what keeps the two entry points from disagreeing, which is the shape of the reported defect.removePresetalready reconciles a gateway-held preset without a registry entry, so the CLI guard was the only obstacle. Covered byremoves a preset the gateway enforces but the registry never recorded,offers a gateway-only preset in the removal picker, andlists a preset both sources hold only once in the removal picker.nullfromgetGatewayPresetsmeans the gateway could not be queried, not that the preset is absent, so the command falls back to the registry and names the evidence it had instead of asserting the preset is not applied. Covered bynames the unreachable gateway when it refuses on local state alone; the unchanged refusal path is covered byrefuses a preset neither the registry nor the gateway holds.applyPresetContentinsrc/lib/policy/index.tswarns when the sandbox has no registry entry instead of returningtruein silence. It keeps the successful gateway mutation, because a built-in preset stays discoverable from the gateway and is now removable; the custom arm still returnsfalse, because a custom preset is discoverable only through the registry. Covered bywarns but keeps the mutation when a built-in preset cannot be recorded locally.src/lib/actions/sandbox/policy-channel-refresh.test.tsandpolicy-channel-lock.test.tsgain agetGatewayPresetsstub. Without it the new gateway read inpolicy removewould make those unit tests spawn a realopenshell policy get.docs/network-policy/apply-policy-presets.mdxdocuments removing a preset the gateway enforces without a local record, the unreachable-gateway behavior, and the unrecorded-sandbox case for maintained and custom presets.docs/reference/commands.mdxupdates thepolicy addandpolicy removereference sections to match.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/network-policy/apply-policy-presets.mdx,docs/reference/commands.mdx. The review returned request-changes. Its blocking finding was that the persistence paragraph stated the built-in outcome without scoping it, so a reader applying a custom preset with--from-fileto an unregistered sandbox would expect a gateway-active preset when the command in fact exits non-zero and the preset appears nowhere. It also found the quotedpolicy listrow indented two spaces whereformatPolicyListPresetRowemits four, no command block before theExpected output:lead-in, the gateway named without its OpenShell qualifier on first use, an unreachable-gateway sentence that did not scope its message to the named form, and both reference sections indocs/reference/commands.mdxleft describing the previous behavior. All findings were verified against source and applied.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run src/lib/actions/sandbox/policy-channel-policy.test.ts src/lib/actions/sandbox/policy-channel-refresh.test.ts src/lib/actions/sandbox/policy-channel-lock.test.ts src/lib/actions/sandbox/policy-channel-list.test.ts— 4 files, 81 tests passed;npx vitest run --project integration test/policies.test.ts— 1 file, 76 tests passed. Reverting only the two changed source files fails 5 of the 6 new cases, so they guard the behavior rather than restate it; the sixth is the preserved refusal path and passes either way by design.npm run typecheck:cliandnpm run lintare clean. The twofern checkwarnings are pre-existing — rebuilding with the doc changes stashed reports the same two.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation