fix(cli): support non-interactive policy preset updates (Fixes #2067)#2070
fix(cli): support non-interactive policy preset updates (Fixes #2067)#2070ericksoa merged 2 commits intoNVIDIA:mainfrom
Conversation
Fixes NVIDIA#2067 Signed-off-by: Deepak Jain <[email protected]>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughEnable non-interactive/scripted execution for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
ericksoa
left a comment
There was a problem hiding this comment.
Clean, focused feature addition. Code is correct and well-tested (6 new tests covering --yes, NEMOCLAW_NON_INTERACTIVE, and fail-fast paths for both add and remove).
One minor nit: --force is accepted in code but not shown in help() — either document it or drop it to avoid hidden API surface.
CI hasn't fully run yet (only check-pr-limit and CodeRabbit completed) — please ensure the checks workflow passes before merging.
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
PR NVIDIA#2070 changed nemoclaw policy-add to require a preset name argument in non-interactive mode. Replace the expect-based interactive approach with direct CLI calls: nemoclaw <sandbox> policy-add <preset> --yes. Add apply_preset_interactive() for TC-NET-03 to also cover the interactive code path (unsets NEMOCLAW_NON_INTERACTIVE, uses expect). Fix TC-NET-03 assertion: accept any HTTP status (2xx-4xx) as reachable since Telegram returns 403 to unauthenticated requests — the endpoint is reachable through the proxy, Telegram itself rejects the bare request. Fixes NVIDIA#2148 Verified on CI: 10/10 PASS on ubuntu-latest. Signed-off-by: Truong Nguyen <[email protected]> Made-with: Cursor
#2149) ## Summary Fix 4 nightly failures in `test/e2e/test-network-policy.sh` caused by PR #2070 changing `nemoclaw policy-add` to require a preset name in non-interactive mode. ## Related Issue Fixes #2148 ## Changes - **Replace expect-based preset application with direct CLI calls:** `apply_preset` now calls `nemoclaw <sandbox> policy-add <preset> --yes` directly instead of capturing the interactive list, grepping for a number, and automating prompts with `expect`. This works with the new non-interactive mode from #2070. - **Add interactive mode coverage (TC-NET-03):** New `apply_preset_interactive` function unsets `NEMOCLAW_NON_INTERACTIVE` and uses `expect` to exercise the interactive prompt path. TC-NET-03 tries interactive first, falls back to non-interactive. Both CLI code paths are now tested. - **Fix TC-NET-03 reachability assertion:** After applying the telegram preset, `api.telegram.org` returns HTTP 403 (unauthenticated — no bot token). This is correct: the proxy lets the request through, Telegram rejects the bare request. Changed assertion to accept any HTTP status (2xx-4xx) as "reachable" — only `ERROR_` responses (proxy blocks) count as failure. - **Remove expect as hard dependency:** `expect` is installed with a warning if missing (only needed for TC-NET-03 interactive test). Non-interactive tests work without it. ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - `npx prek run --all-files` passes - Tests added or updated for new or changed behavior - No secrets, API keys, or credentials committed - Verified on CI: 10/10 PASS on ubuntu-latest ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen <[email protected]> Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Tests now tolerate missing interactive tooling and log a warning instead of aborting. * Added non-interactive and interactive preset-application flows with automatic fallback when interactive tooling is unavailable. * Simplified one preset workflow to a direct dry-run path that checks for "would be opened" output. * Improved result classification to better distinguish success, proxy-blocked, and unexpected responses. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Truong Nguyen <[email protected]> Co-authored-by: Carlos Villela <[email protected]>
#2149) ## Summary Fix 4 nightly failures in `test/e2e/test-network-policy.sh` caused by PR #2070 changing `nemoclaw policy-add` to require a preset name in non-interactive mode. ## Related Issue Fixes #2148 ## Changes - **Replace expect-based preset application with direct CLI calls:** `apply_preset` now calls `nemoclaw <sandbox> policy-add <preset> --yes` directly instead of capturing the interactive list, grepping for a number, and automating prompts with `expect`. This works with the new non-interactive mode from #2070. - **Add interactive mode coverage (TC-NET-03):** New `apply_preset_interactive` function unsets `NEMOCLAW_NON_INTERACTIVE` and uses `expect` to exercise the interactive prompt path. TC-NET-03 tries interactive first, falls back to non-interactive. Both CLI code paths are now tested. - **Fix TC-NET-03 reachability assertion:** After applying the telegram preset, `api.telegram.org` returns HTTP 403 (unauthenticated — no bot token). This is correct: the proxy lets the request through, Telegram rejects the bare request. Changed assertion to accept any HTTP status (2xx-4xx) as "reachable" — only `ERROR_` responses (proxy blocks) count as failure. - **Remove expect as hard dependency:** `expect` is installed with a warning if missing (only needed for TC-NET-03 interactive test). Non-interactive tests work without it. ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - `npx prek run --all-files` passes - Tests added or updated for new or changed behavior - No secrets, API keys, or credentials committed - Verified on CI: 10/10 PASS on ubuntu-latest ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen <[email protected]> Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Tests now tolerate missing interactive tooling and log a warning instead of aborting. * Added non-interactive and interactive preset-application flows with automatic fallback when interactive tooling is unavailable. * Simplified one preset workflow to a direct dry-run path that checks for "would be opened" output. * Improved result classification to better distinguish success, proxy-blocked, and unexpected responses. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Truong Nguyen <[email protected]> Co-authored-by: Carlos Villela <[email protected]>
## Summary Catches up the user-facing reference and troubleshooting docs with the CLI and policy behavior changes that landed in v0.0.21. Drafted via the `nemoclaw-contributor-update-docs` skill against commits in `v0.0.20..v0.0.21`, filtered through `docs/.docs-skip`. ## Changes - **`docs/reference/commands.md`** - `nemoclaw list`: session indicator (●) for connected sandboxes (#2117). - `nemoclaw <name> connect`: active-session note; auto-recovery from SSH identity drift after a host reboot (#2117, #2064). - `nemoclaw <name> status`: three-state Inference line (`healthy` / `unreachable` / `not probed`) covering both local and remote providers; new `Connected` line (#2002, #2117). - `nemoclaw <name> destroy` and `rebuild`: active-session warning with second confirm; rebuild reapplies policy presets to the recreated sandbox (#2117, #2026). - `nemoclaw <name> policy-add` and `policy-remove`: positional preset argument and non-interactive flow via `--yes`/`--force`/`NEMOCLAW_NON_INTERACTIVE=1` (#2070). - `nemoclaw <name> policy-list`: registry-vs-gateway desync detection (#2089). - **`docs/reference/troubleshooting.md`** - `Reconnect after a host reboot`: now reflects automatic stale `known_hosts` pruning on `connect` (#2064). - `Running multiple sandboxes simultaneously`: onboard's forward-port collision guard (#2086). - New section: `openclaw config set` or `unset` is blocked inside the sandbox (#2081). - **`docs/network-policy/customize-network-policy.md`**: non-interactive `policy-add`/`policy-remove` form; preset preservation across rebuild (#2070, #2026). - **`docs/inference/use-local-inference.md`**: NIM section now covers the NGC API key prompt with masked input and `docker login nvcr.io --password-stdin` behavior (#2043). - **Generated skills regenerated** to pick up the source changes (`.agents/skills/nemoclaw-user-reference/references/{commands,troubleshooting}.md`, plus minor heading-flow deltas elsewhere). The pre-commit `Regenerate agent skills from docs` hook ran and confirmed source ↔ generated parity. Commits skipped per `docs/.docs-skip` or no doc impact: `bbbaa0fb` (skip-features), `7cb482cb` (skip-features), `8dee23fd` (skip-terms), plus the usual CI / test / refactor / install-plumbing churn. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes for the modified files (the one failing test, `test/cli.test.ts > unknown command exits 1`, also fails on `origin/main` and is unrelated to these markdown-only changes) - [ ] `npm test` passes — skipped; same pre-existing CLI-dispatch test failure unrelated to docs - [ ] Tests added or updated for new or changed behavior — n/a, doc-only - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) — not run locally - [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) — n/a, no new pages ## AI Disclosure - [x] AI-assisted — tool: Claude Code --- Signed-off-by: Miyoung Choi <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Multi-session SSH connections with concurrent session support. * Three-state inference health reporting (healthy/unreachable/not probed) across all providers. * Automatic SSH host key rotation detection and recovery. * Non-interactive policy preset management via positional arguments. * Session indicators in sandbox list view. * **Bug Fixes** * Protected destructive operations with active-session warnings. * Policy presets now preserved during sandbox rebuilds. * **Documentation** * NGC registry authentication requirements for container images. * Multi-sandbox onboarding and reconnection guidance. * Troubleshooting updates for port conflicts and SSH issues. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Summary
nemoclaw <sandbox> policy-addandpolicy-removewere prompt-only flows, which made them unusable from headless shells, CI, andNEMOCLAW_NON_INTERACTIVE=1paths. This change adds an explicit preset-argument form so operators can script built-in preset changes withoutexpector an interactive TTY.Changes
src/nemoclaw.ts- acceptpolicy-add <preset>andpolicy-remove <preset>; validate the preset name; support--yesandNEMOCLAW_NON_INTERACTIVE=1for the explicit-preset path; fail fast with a clear usage message when non-interactive mode is requested without a preset name; update CLI help text to show the new form.test/policies.test.ts- add regression coverage for scripted add/remove with--yes, for theNEMOCLAW_NON_INTERACTIVE=1path, and for the new fast-fail error when non-interactive mode is used without a preset name.Testing
npm run build:clinpm run typecheck:clipolicy-add pypi --yesapplies without promptingNEMOCLAW_NON_INTERACTIVE=1 policy-remove pypiremoves without promptingnpm testsrc/lib/sandbox-version.test.ts,src/lib/preflight.test.ts,test/install-preflight.test.ts, andtest/legacy-path-guard.test.tsFixes #2067
Summary by CodeRabbit
New Features
--yes/--forceflags or an env var, allowing automated apply/remove operations.Tests