Skip to content

test(channels): cover remove and stop dry-run flows - #5484

Merged
cv merged 1 commit into
mainfrom
test/policy-channel-remove-coverage
Jun 16, 2026
Merged

test(channels): cover remove and stop dry-run flows#5484
cv merged 1 commit into
mainfrom
test/policy-channel-remove-coverage

Conversation

@cv

@cv cv commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds focused coverage for channel remove and stop command guardrails. The tests exercise usage errors and dry-run behavior without mutating gateway or registry state.

Changes

  • Added src/lib/actions/sandbox/policy-channel-remove-flow.test.ts.
  • Covered channels remove with missing channel arguments.
  • Covered channels remove --dry-run and channels stop --dry-run output and no-exit behavior.

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

  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
    • npx vitest run src/lib/actions/sandbox/policy-channel-remove-flow.test.ts --project cli
    • npm run typecheck:cli
  • Full npm test passes (broad runtime changes only)
  • 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)

Note: commit and push hooks were skipped for this test-only branch because the broad local hook suite has unrelated pre-existing CLI failures in this checkout; targeted tests and CLI typecheck passed.


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

Summary by CodeRabbit

  • Tests
    • Added comprehensive test coverage for sandbox policy channel removal and disable flows, ensuring proper error handling and dry-run behavior validation.

@cv cv self-assigned this Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

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: 93c1d2a5-9a2e-48d4-9d72-20396ca3baf7

📥 Commits

Reviewing files that changed from the base of the PR and between 2e8e519 and a48e472.

📒 Files selected for processing (1)
  • src/lib/actions/sandbox/policy-channel-remove-flow.test.ts

📝 Walkthrough

Walkthrough

A new Vitest test file is added for the sandbox policy-channel module. It covers three control-flow scenarios: exiting with code 1 when no channel is supplied to removeSandboxChannel, dry-run removal with an explicit channel, and dry-run stop with stubbed registry queries (getSandbox, getConfiguredMessagingChannelsFromEntry, getDisabledChannels).

Changes

Sandbox policy-channel remove/stop flow tests

Layer / File(s) Summary
Vitest suite for remove/stop control flows
src/lib/actions/sandbox/policy-channel-remove-flow.test.ts
Adds three test cases: (1) removeSandboxChannel exits with code 1 when no channel argument is provided; (2) dry-run removal with an explicit channel logs the --dry-run message and does not call process.exit; (3) dry-run stop with mocked registry queries (getSandbox, getConfiguredMessagingChannelsFromEntry, getDisabledChannels) logs the expected message and does not call process.exit. The module is loaded via createRequire and the require cache is cleared between tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

v0.0.66

Poem

🐇 Hop, hop, the channels align,
A dry-run whisper, no exit this time!
The cache is cleared, the mocks are set,
Three tests run clean with no regret.
No channel? Exit one! That's the law—
The rabbit checks each edge with a paw. 🌿

🚥 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 accurately summarizes the main change: adding test coverage for channel remove and stop dry-run flows, which matches the PR's focus on testing these command scenarios.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/policy-channel-remove-coverage

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

@github-code-quality

github-code-quality Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the test/policy-channel-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main test/policy-channel-... a48e472 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the test/policy-channel-... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main test/policy-channel-... a48e472 +/-
src/lib/state/o...oard-session.ts 90%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/onboard/preflight.ts 64%
src/lib/actions...dbox/rebuild.ts 62%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 17%

Updated June 16, 2026 00:33 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

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 this is a tests-only change adding unit coverage for existing policy-channel behavior, with no runtime or user-flow code changes that could affect installer/onboarding, sandbox lifecycle, credentials, security boundaries, network policy, inference routing, deployment, or real assistant flows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. The PR only adds a non-scenario Vitest test outside test/e2e-scenario/ and does not change the shared Vitest E2E scenario workflow or scenario-relevant files, so no Vitest E2E scenario dispatch is indicated.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • None.

@github-actions

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Assert dry-run paths do not mutate channel state (src/lib/actions/sandbox/policy-channel-remove-flow.test.ts:45): The new dry-run tests assert the user-facing output and that process.exit is not called, but they do not directly verify the safety property described by the PR: no gateway, registry, manifest, policy, token, durable-state, or rebuild side effects. Because channel remove/stop flows are security-sensitive policy and sandbox lifecycle surfaces, a future regression could still mutate state during --dry-run while keeping these tests green.
    • Recommendation: Add explicit negative assertions around the side-effect boundaries used by removeSandboxChannel and stopSandboxChannel, or spy on the relevant compiled dependency exports before loading the SUT. Cover token clearing, durable state cleanup, gateway/registry mutation, messaging plan persistence, policy preset removal, and rebuild prompting as applicable.
    • Evidence: The remove dry-run test at lines 42-52 only checks the dry-run log text and exitSpy. The stop dry-run test at lines 55-68 only mocks registry reads, checks log text, and checks exitSpy. No side-effect helpers are asserted as not called.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Mocked behavioral coverage** — removeSandboxChannel --dry-run does not clear channel tokens or durable sandbox state. The added tests are appropriate unit coverage for visible CLI guardrails, but the dry-run safety claim would be stronger with explicit negative assertions at the mocked side-effect boundaries.
  • **Mocked behavioral coverage** — removeSandboxChannel --dry-run does not mutate the OpenShell gateway or registry entry. The added tests are appropriate unit coverage for visible CLI guardrails, but the dry-run safety claim would be stronger with explicit negative assertions at the mocked side-effect boundaries.
  • **Mocked behavioral coverage** — removeSandboxChannel --dry-run does not remove the channel policy preset or persist a messaging-plan removal. The added tests are appropriate unit coverage for visible CLI guardrails, but the dry-run safety claim would be stronger with explicit negative assertions at the mocked side-effect boundaries.
  • **Mocked behavioral coverage** — removeSandboxChannel --dry-run does not prompt for or start a rebuild. The added tests are appropriate unit coverage for visible CLI guardrails, but the dry-run safety claim would be stronger with explicit negative assertions at the mocked side-effect boundaries.
  • **Mocked behavioral coverage** — stopSandboxChannel --dry-run does not persist a disabled-channel plan or prompt for rebuild. The added tests are appropriate unit coverage for visible CLI guardrails, but the dry-run safety claim would be stronger with explicit negative assertions at the mocked side-effect boundaries.
  • **Assert dry-run paths do not mutate channel state** — Add explicit negative assertions around the side-effect boundaries used by removeSandboxChannel and stopSandboxChannel, or spy on the relevant compiled dependency exports before loading the SUT. Cover token clearing, durable state cleanup, gateway/registry mutation, messaging plan persistence, policy preset removal, and rebuild prompting as applicable.
  • **Acceptance clause:** The tests exercise usage errors and dry-run behavior without mutating gateway or registry state. — add test evidence or identify existing coverage. Usage errors and dry-run output/no-exit behavior are exercised, but the tests do not directly assert that gateway, registry, policy, manifest, token, durable-state, or rebuild side effects are skipped.
Since last review details

Current findings:

  • Assert dry-run paths do not mutate channel state (src/lib/actions/sandbox/policy-channel-remove-flow.test.ts:45): The new dry-run tests assert the user-facing output and that process.exit is not called, but they do not directly verify the safety property described by the PR: no gateway, registry, manifest, policy, token, durable-state, or rebuild side effects. Because channel remove/stop flows are security-sensitive policy and sandbox lifecycle surfaces, a future regression could still mutate state during --dry-run while keeping these tests green.
    • Recommendation: Add explicit negative assertions around the side-effect boundaries used by removeSandboxChannel and stopSandboxChannel, or spy on the relevant compiled dependency exports before loading the SUT. Cover token clearing, durable state cleanup, gateway/registry mutation, messaging plan persistence, policy preset removal, and rebuild prompting as applicable.
    • Evidence: The remove dry-run test at lines 42-52 only checks the dry-run log text and exitSpy. The stop dry-run test at lines 55-68 only mocks registry reads, checks log text, and checks exitSpy. No side-effect helpers are asserted as not called.

Workflow run details

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

@cv
cv merged commit 01c5d0b into main Jun 16, 2026
46 checks passed
@cv
cv deleted the test/policy-channel-remove-coverage branch June 16, 2026 00:44
@cv cv added the v0.0.66 label Jun 17, 2026
@wscurran wscurran added the chore Build, CI, dependency, or tooling maintenance label Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants