Skip to content

fix(policy): report custom presets that cannot be recorded locally#4576

Open
latenighthackathon wants to merge 1 commit into
NVIDIA:mainfrom
latenighthackathon:fix/policy-add-from-file-persist-4510
Open

fix(policy): report custom presets that cannot be recorded locally#4576
latenighthackathon wants to merge 1 commit into
NVIDIA:mainfrom
latenighthackathon:fix/policy-add-from-file-persist-4510

Conversation

@latenighthackathon
Copy link
Copy Markdown
Contributor

@latenighthackathon latenighthackathon commented May 31, 2026

Summary

nemoclaw <sandbox> policy-add --from-file <preset> applies a custom preset to the gateway and exits 0, but the preset never appears in policy-list or status. applyPresetContent records a custom preset under the sandbox's customPolicies only when the sandbox has a local registry entry, yet it returns true regardless, so a sandbox that is missing from the registry produces a silent success with nothing recorded.

Related Issue

Fixes #4510

Changes

  • applyPresetContent (src/lib/policy/index.ts) now returns false and warns when a custom preset reaches the gateway but cannot be recorded locally because the sandbox has no registry entry. policy-add --from-file already exits non-zero on a false result, so the command no longer reports success for a preset that policy-list and status cannot display.
  • The built-in preset path is unchanged: built-in presets are matched against the live gateway by name (getGatewayPresets iterates listPresets()), so they stay visible without a registry entry; only custom presets depend on the registry record (listCustomPresets and getGatewayPresets read registry.getCustomPolicies).
  • Added regression coverage in test/policies.test.ts for the unrecorded case (returns false, warns) and the registered-sandbox case (records the preset, returns true).

Type of Change

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

Verification

  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed

Ran npm run build:cli, npm run typecheck:cli, npx @biomejs/biome lint, and the policies and sandbox/policy vitest suites (157 passing, including the new regression tests, which fail before the change and pass after).

Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for custom preset application: now correctly detects and warns when a preset is applied to the gateway but cannot be recorded locally, with guidance for users to verify their policies.
  • Tests

    • Added regression tests for custom preset recording when target sandbox is missing from the registry.

applyPresetContent applied a custom preset to the gateway and then
returned true even when the sandbox had no local registry entry, so
addCustomPolicy never ran and the preset was not recorded under
customPolicies. Because policy-list and status surface custom presets
only from the registry (listCustomPresets and getGatewayPresets both
read registry.getCustomPolicies), the preset stayed invisible while
policy-add --from-file still exited 0.

Return false and warn when a custom preset reaches the gateway but
cannot be recorded locally, so the command no longer reports success
for a preset that policy-list and status cannot show.

Fixes NVIDIA#4510

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 31, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 31, 2026

Review Change Stack

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: cf40a623-e0fd-4e12-8b5b-ff09765783a5

📥 Commits

Reviewing files that changed from the base of the PR and between a25b393 and ec74a85.

📒 Files selected for processing (2)
  • src/lib/policy/index.ts
  • test/policies.test.ts

📝 Walkthrough

Walkthrough

applyPresetContent now detects when a custom preset is successfully applied to the gateway but the target sandbox has no local registry entry. When this occurs, the function logs a warning and returns false instead of true, preventing the operation from being incorrectly reported as fully applied. Regression tests validate both the missing-sandbox error path and the normal success path.

Changes

Missing Sandbox Detection and Fallback

Layer / File(s) Summary
Missing sandbox detection and regression test
src/lib/policy/index.ts, test/policies.test.ts
When registry.getSandbox(sandboxName) returns null for a custom preset, the function logs a warning, does not record the preset locally, and returns false. Regression test for issue 4510 mocks registry sandbox lookup and validates both the missing-sandbox error path (returns false, no addCustomPolicy call) and the normal success path (returns true, calls addCustomPolicy correctly).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4013: This PR directly affects session policyPresets syncing logic in the retrieved PR, which proceeds only when applyPresetContent succeeds.
  • NVIDIA/NemoClaw#4228: Both PRs modify applyPresetContent in src/lib/policy/index.ts and its test coverage in test/policies.test.ts.

Suggested labels

fix, bug, NemoClaw CLI, v0.0.53

Suggested reviewers

  • cv
  • ericksoa

Poem

🐰 A sandbox vanished, slipped from the local store,
Yet the gateway accepted, so what was in store?
Now we check if it's missing and warn what occurred,
The truth in false returned—now the error is heard! 📋

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: ensuring custom presets that cannot be recorded locally are reported as failures rather than silent successes.
Linked Issues check ✅ Passed The PR changes fully address issue #4510's requirements: applyPresetContent now returns false and logs warnings when custom presets cannot be recorded locally due to missing registry entries, preventing false success exits.
Out of Scope Changes check ✅ Passed All changes are directly within scope: modifications to applyPresetContent behavior for unrecorded custom presets and comprehensive regression tests covering both success and failure paths.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][Policy] policy-add --from-file exits 0 but custom preset is absent from policy-list and status

1 participant