test(e2e): remove duplicate channels remove coverage

[sandl99]

Summary

Remove the duplicated channels remove phase from the channels stop/start E2E so removal coverage stays in channels-add-remove-e2e. Update the nightly E2E workflow metadata to match the shortened stop/start job.

Changes

• Removed the live channels remove loop, remove-only helpers, and post-remove rebuild assertions from test/e2e/test-channels-stop-start.sh.
• Updated the stop/start E2E script header so it no longer advertises [Channels] channels remove <ch> fails on a live sandbox and leaves the channel's policy preset applied #3671 removal coverage.
• Renamed the nightly workflow job text from stop/start/remove to stop/start while leaving add/remove coverage documented in the add/remove job.

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
• make 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: San Dang sdang@nvidia.com

Summary by CodeRabbit

• Tests

  • Narrowed end-to-end test scope to stop/start-only lifecycle regression testing.
  • Removed channel removal scenario and related helper utilities, simplifying flows and assertions.
  • Streamlined execution to stop-all, rebuild, start-all and rebuild verification.

• Documentation

  • Updated CI job comments to reflect stop/start-only coverage and clarified referenced regressions.

[coderabbitai]

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: 5f3472bc-8f37-40ed-953a-23ec7a9dbc31

📥 Commits

Reviewing files that changed from the base of the PR and between ce2634b and 8ca1615.

📒 Files selected for processing (1)

• .github/workflows/nightly-e2e.yaml

---

📝 Walkthrough

Walkthrough

CI workflow comments and the E2E test were narrowed to a stop/start-only lifecycle: test header and job comments updated; helper functions and the live “channels remove” scenario (and its assertions) were removed so the script runs stop-all + rebuild + start-all + rebuild.

Changes

E2E Test Scope Narrowing

Layer / File(s)	Summary
CI workflow and test scope documentation .github/workflows/nightly-e2e.yaml, test/e2e/test-channels-stop-start.sh	Workflow job comments and the test header/step label were changed to remove #3671 remove-scenario coverage and state the test covers stop/start regressions (#3453, #3381) only.
Removal of channels-remove helpers and scenario section test/e2e/test-channels-stop-start.sh	Removed helper functions registry_object_has_key, token_keys_for_channel, assert_channel_providers_deleted, assert_channel_hashes_absent, and remove_all_channels. Deleted the agent scenario section that ran channels remove all and its rebuild/verification steps; the script now runs stop-all + rebuild + start-all + rebuild verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4013: Related work touching the channels remove remove/rebuild behavior and registry/preset synchronization targeted by the removed test coverage.

Suggested reviewers

• ericksoa

Poem

🐰 I hopped through tests with careful art,
Snipped the remove, kept stop/start smart,
Helpers pruned like springtime green,
Agent runs lean, the logs stay clean,
A tiny rabbit cheers the new green start.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'test(e2e): remove duplicate channels remove coverage' directly and clearly summarizes the main change: removing duplicate channels remove coverage from the e2e test suite.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch u/sdang/remove-channel-remove-from-stop-start-e2e

---

_{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: 0 prior items resolved, 1 still applies, 0 new items found

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Removed channel-remove coverage is still not fully duplicated (test/e2e/test-channels-stop-start.sh:626): The PR removes the live `channels remove` phase after the OpenClaw and Hermes stop/start scenarios. The remaining add/remove E2E is Telegram-only and OpenClaw-oriented, while the deleted stop/start phase covered removal after all channels had been active for both agents and included provider deletion, registry channel state, credential-hash cleanup, policy-preset cleanup, and post-remove rebuild checks across telegram, discord, wechat, slack, and whatsapp. This leaves less runtime coverage for security-relevant cleanup paths such as stale gateway providers, credential hash state, and network policy presets.
  • Recommendation: Either retain a smaller representative remove phase in `test/e2e/test-channels-stop-start.sh`, or extend `test/e2e/test-channels-add-remove.sh` so it covers the removed surfaces: Hermes, non-Telegram channels, Slack's multiple providers, registry messaging/disabled state, provider credential hash cleanup, policy un-apply, and rebuild with token env still present.
  • Evidence: Current stop/start scenario now ends after `assert_provider_records_exist "after start"` at line 626. The diff deletes `remove_all_channels`, `assert_channel_providers_deleted`, `assert_channel_hashes_absent`, and the post-remove rebuild assertions. `test/e2e/test-channels-add-remove.sh` states `Telegram-only — Discord/Slack walk the same KNOWN_CHANNELS + preset lookup code path` and only invokes `channels remove telegram`; its post-remove assertions check OpenClaw telegram config/provider/policy cleanup, not Hermes or the full multi-channel matrix. This is unchanged from the previous advisor review, so that prior finding still applies.

🌱 Nice ideas

• None.

Since last review details

Current findings:

• Removed channel-remove coverage is still not fully duplicated (test/e2e/test-channels-stop-start.sh:626): The PR removes the live `channels remove` phase after the OpenClaw and Hermes stop/start scenarios. The remaining add/remove E2E is Telegram-only and OpenClaw-oriented, while the deleted stop/start phase covered removal after all channels had been active for both agents and included provider deletion, registry channel state, credential-hash cleanup, policy-preset cleanup, and post-remove rebuild checks across telegram, discord, wechat, slack, and whatsapp. This leaves less runtime coverage for security-relevant cleanup paths such as stale gateway providers, credential hash state, and network policy presets.
  • Recommendation: Either retain a smaller representative remove phase in `test/e2e/test-channels-stop-start.sh`, or extend `test/e2e/test-channels-add-remove.sh` so it covers the removed surfaces: Hermes, non-Telegram channels, Slack's multiple providers, registry messaging/disabled state, provider credential hash cleanup, policy un-apply, and rebuild with token env still present.
  • Evidence: Current stop/start scenario now ends after `assert_provider_records_exist "after start"` at line 626. The diff deletes `remove_all_channels`, `assert_channel_providers_deleted`, `assert_channel_hashes_absent`, and the post-remove rebuild assertions. `test/e2e/test-channels-add-remove.sh` states `Telegram-only — Discord/Slack walk the same KNOWN_CHANNELS + preset lookup code path` and only invokes `channels remove telegram`; its post-remove assertions check OpenClaw telegram config/provider/policy cleanup, not Hermes or the full multi-channel matrix. This is unchanged from the previous advisor review, so that prior finding still applies.

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: channels-stop-start-e2e, channels-add-remove-e2e

Dispatch hint: channels-stop-start-e2e,channels-add-remove-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No merge-blocking E2E is required because the PR only changes E2E test coverage and workflow comments, not installer, onboarding, sandbox lifecycle implementation, credentials handling, network policy implementation, inference routing, deployment code, or user-facing assistant runtime behavior.

Optional E2E

• channels-stop-start-e2e (high): Validates the directly modified E2E script still exercises OpenClaw and Hermes channel stop/start lifecycle across telegram, discord, wechat, slack, and whatsapp after the coverage split.
• channels-add-remove-e2e (medium): Useful adjacent confidence that the live add/remove coverage removed from test-channels-stop-start.sh remains covered by the separate add/remove lifecycle job referenced in the workflow comments.

New E2E recommendations

• None.

Dispatch hint

• Workflow: .github/workflows/nightly-e2e.yaml
• jobs input: channels-stop-start-e2e,channels-add-remove-e2e

[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 workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

• None.

Relevant changed files

• None.

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 26433504986
Target ref: u/sdang/remove-channel-remove-from-stop-start-e2e
Workflow ref: u/sdang/remove-channel-remove-from-stop-start-e2e
Requested jobs: channels-stop-start-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job	Result
channels-stop-start-e2e	✅ success