test(policy): move confirmation coverage to action tests

[cv]

Summary

This PR targets the next slow policy runtime bucket identified after #4919 by moving built-in policy-add / policy-remove confirmation coverage out of subprocess-heavy CLI harnesses and into same-process action tests. Custom preset --from-file / --from-dir subprocess coverage stays in test/policies.test.ts for a smaller, mechanical first step.

Related Issue

Part of #4892

Changes

• Add src/lib/actions/sandbox/policy-channel-policy.test.ts with mocked same-process coverage for addSandboxPolicy and removeSandboxPolicy prompts, declined confirmation, dry-run, non-interactive mode, agent-specific preset filtering, and messaging validation guidance.
• Remove the equivalent built-in policy-add/remove subprocess blocks from test/policies.test.ts, reducing the policy suite from 166 tests in the test(cli): trim logs subprocess coverage #4919 artifact to 144 tests in this full local run.
• Ratchet test/policies.test.ts in ci/test-file-size-budget.json from 3143 lines to 2763 lines so it cannot silently grow back.
• Leave custom preset --from-file / --from-dir subprocess coverage intact for a follow-up target.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Carlos Villela cvillela@nvidia.com

[coderabbitai]

Warning

Review limit reached

@cv, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 5 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0c4d27c1-82dc-40bb-87ea-4dd8010da6b7

📥 Commits

Reviewing files that changed from the base of the PR and between 0cb2c84 and eb8c67d.

📒 Files selected for processing (3)

• ci/test-file-size-budget.json
• src/lib/actions/sandbox/policy-channel-policy.test.ts
• test/policies.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/policy-runtime-target

---

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

[github-actions]

PR Review Advisor

Findings: 0 needs attention, 1 worth checking, 0 nice ideas
Since last review: 1 prior item resolved, 0 still apply, 1 new item found

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Public CLI policy mutation coverage is reduced to direct action calls (src/lib/actions/sandbox/policy-channel-policy.test.ts:126): The moved tests now call addSandboxPolicy/removeSandboxPolicy directly with mocked modules. That preserves most action behavior coverage, but the deleted subprocess tests also exercised the sandbox-first public CLI path (`nemoclaw <sandbox> policy-add/remove ...`) through argument dispatch, environment handling, and action invocation. Policy add/remove is a security-sensitive policy mutation boundary, so a dispatch or parser regression could now pass these new action tests.
  • Recommendation: Keep a small subprocess or dispatcher-level regression test for the legacy sandbox-first built-in flows, especially `policy-add <preset> --yes`, `policy-remove <preset> -y`, and non-interactive mode without a preset. The existing oclif command mapping test helps, but it does not fully replace the removed public CLI path coverage.
  • Evidence: New direct action tests cover confirmations and options in `src/lib/actions/sandbox/policy-channel-policy.test.ts:126-328`; removed blocks in `test/policies.test.ts` previously spawned the CLI for built-in `policy-add`/`policy-remove`; nearby `src/commands/sandbox/policy/mutate.test.ts:39` covers native command `-y` mapping but not full sandbox-first dispatch.

🌱 Nice ideas

• None.

Consider writing more tests for

• **Runtime validation** — legacy sandbox-first `policy-add <preset> --yes` dispatches to `addSandboxPolicy` without prompting. The moved unit-style action tests cover most behavior, but policy add/remove are security-sensitive CLI policy mutation paths and the PR removes subprocess coverage of the public sandbox-first CLI boundary. The size-budget change also benefits from a targeted validation that the ratchet is enforced.
• **Runtime validation** — legacy sandbox-first `policy-remove <preset> -y` dispatches to `removeSandboxPolicy` without prompting. The moved unit-style action tests cover most behavior, but policy add/remove are security-sensitive CLI policy mutation paths and the PR removes subprocess coverage of the public sandbox-first CLI boundary. The size-budget change also benefits from a targeted validation that the ratchet is enforced.
• **Runtime validation** — legacy sandbox-first `policy-add` with `NEMOCLAW_NON_INTERACTIVE=1` exits before selecting or applying a preset when no preset is supplied. The moved unit-style action tests cover most behavior, but policy add/remove are security-sensitive CLI policy mutation paths and the PR removes subprocess coverage of the public sandbox-first CLI boundary. The size-budget change also benefits from a targeted validation that the ratchet is enforced.
• **Runtime validation** — test-size budget check rejects `test/policies.test.ts` growing above the new 2763-line limit. The moved unit-style action tests cover most behavior, but policy add/remove are security-sensitive CLI policy mutation paths and the PR removes subprocess coverage of the public sandbox-first CLI boundary. The size-budget change also benefits from a targeted validation that the ratchet is enforced.
• **Public CLI policy mutation coverage is reduced to direct action calls** — Keep a small subprocess or dispatcher-level regression test for the legacy sandbox-first built-in flows, especially `policy-add <preset> --yes`, `policy-remove <preset> -y`, and non-interactive mode without a preset. The existing oclif command mapping test helps, but it does not fully replace the removed public CLI path coverage.
• **Acceptance clause:** This PR targets the next slow policy runtime bucket identified after test(cli): trim logs subprocess coverage #4919 by moving built-in `policy-add` / `policy-remove` confirmation coverage out of subprocess-heavy CLI harnesses and into same-process action tests. — add test evidence or identify existing coverage. The built-in confirmation tests were moved into `src/lib/actions/sandbox/policy-channel-policy.test.ts`, but this also removes some public CLI subprocess-path coverage for policy mutations.
• **Acceptance clause:** Remove the equivalent built-in policy-add/remove subprocess blocks from `test/policies.test.ts`, reducing the policy suite from 166 tests in the test(cli): trim logs subprocess coverage #4919 artifact to 144 tests in this full local run. — add test evidence or identify existing coverage. The built-in subprocess blocks are removed from `test/policies.test.ts`; the specific local test-count claim was not independently verified because no commands were run.
• **Acceptance clause:** Part of ci: safely split slow CLI coverage suites #4892 — add test evidence or identify existing coverage. The PR body references ci: safely split slow CLI coverage suites #4892, but the deterministic context provided no linked issue body or comments to extract acceptance clauses from.

Since last review details

Current findings:

• Public CLI policy mutation coverage is reduced to direct action calls (src/lib/actions/sandbox/policy-channel-policy.test.ts:126): The moved tests now call addSandboxPolicy/removeSandboxPolicy directly with mocked modules. That preserves most action behavior coverage, but the deleted subprocess tests also exercised the sandbox-first public CLI path (`nemoclaw <sandbox> policy-add/remove ...`) through argument dispatch, environment handling, and action invocation. Policy add/remove is a security-sensitive policy mutation boundary, so a dispatch or parser regression could now pass these new action tests.
  • Recommendation: Keep a small subprocess or dispatcher-level regression test for the legacy sandbox-first built-in flows, especially `policy-add <preset> --yes`, `policy-remove <preset> -y`, and non-interactive mode without a preset. The existing oclif command mapping test helps, but it does not fully replace the removed public CLI path coverage.
  • Evidence: New direct action tests cover confirmations and options in `src/lib/actions/sandbox/policy-channel-policy.test.ts:126-328`; removed blocks in `test/policies.test.ts` previously spawned the CLI for built-in `policy-add`/`policy-remove`; nearby `src/commands/sandbox/policy/mutate.test.ts:39` covers native command `-y` mapping but not full sandbox-first dispatch.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No E2E is recommended because all changed files are tests or CI test-size metadata. The diff refactors/relocates policy add/remove unit coverage and updates the line-count budget, with no production code, workflow runtime, policy YAML asset, sandbox lifecycle, credentials, inference routing, or onboarding behavior changes.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

• None. No scenario E2E jobs are recommended because this PR only changes non-scenario unit/integration test files and a CI test file-size budget file; it does not modify test/e2e-scenario/, scenario workflows, scenario metadata, expected-state contracts, suite definitions, runtime code, or scenario-relevant helpers.

Optional scenario E2E

• None.

Relevant changed files

• None.