Skip to content

refactor(messaging): clean up some hard code stragglers - #5463

Merged
cv merged 30 commits into
mainfrom
refactor/messaging-channel-manifest-cleanup
Jun 15, 2026
Merged

cv merged 30 commits into
mainfrom
refactor/messaging-channel-manifest-cleanup

Conversation

@sandl99

@sandl99 sandl99 commented Jun 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Clean up remaining concrete messaging-channel references by routing core behavior through manifest metadata and hook registration. This keeps reset help text generic while preserving channel-specific implementation inside channel-owned modules.

Changes

  • Update credentials reset to use a generic <channels> placeholder instead of enumerating concrete messaging channels.
  • Derive OpenClaw managed channel restore ownership and credentialless provider checks from messaging manifests.
  • Route persisted build-step hook outputs through the built-in messaging hook registry instead of special-casing the WeChat seed hook.
  • Add focused metadata and reset coverage for the manifest-derived 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
  • 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)

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Updated credentials reset bridge-retirement guidance to use channels remove <channel> help text.
    • Messaging-bridge verification now determines credentialless channels from channel metadata, not a static list.
  • Refactor

    • Openclaw managed-channel detection is now derived dynamically from channel manifests.
    • Messaging hook output computation was generalized for consistent execution.
  • Tests

    • Expanded tests for credentialless channel and Openclaw managed-channel listing, plus updated config merge/restoration expectations.

sandl99 added 23 commits June 12, 2026 19:05
…annel-manifest-stragglers

# Conflicts:
#	src/lib/actions/sandbox/doctor.ts
#	src/lib/actions/sandbox/policy-channel-conflict.test.ts
#	src/lib/actions/sandbox/policy-channel.ts
#	src/lib/actions/sandbox/snapshot.ts
#	src/lib/messaging/applier/host-state-applier.ts
#	src/lib/messaging/plan-validation.ts
#	src/lib/sandbox/channels.ts
#	src/lib/security/redact.ts
#	test/cli/doctor-gateway-token.test.ts
…annel-manifest-stragglers

# Conflicts:
#	ci/test-file-size-budget.json
…est-stragglers' into refactor/messaging-channel-manifest-stragglers
@sandl99 sandl99 self-assigned this Jun 15, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 15, 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: 7c7b2ff4-a10c-4524-8aab-b5353ac3da17

📥 Commits

Reviewing files that changed from the base of the PR and between 9305d09 and 6b76e17.

📒 Files selected for processing (10)
  • src/lib/messaging/channels/discord/manifest.ts
  • src/lib/messaging/channels/manifests.test.ts
  • src/lib/messaging/channels/metadata.test.ts
  • src/lib/messaging/channels/metadata.ts
  • src/lib/messaging/channels/slack/manifest.ts
  • src/lib/messaging/channels/telegram/manifest.ts
  • src/lib/messaging/channels/wechat/manifest.ts
  • src/lib/messaging/channels/whatsapp/manifest.ts
  • src/lib/messaging/manifest/types.ts
  • src/lib/state/openclaw-config-merge.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/state/openclaw-config-merge.test.ts
  • src/lib/messaging/channels/metadata.test.ts

📝 Walkthrough

Walkthrough

Manifest runtime types are strengthened to support explicit OpenClaw channel names. All five supported messaging channels add channelName fields to their manifests. Two new helper functions in metadata.ts — listMessagingChannelsWithoutCredentials and listOpenClawManagedChannelNames — derive channel lists dynamically from manifests, replacing hardcoded arrays in openclaw-config-merge.ts, verify-deployment.ts, and a WeChat-specific special case in persistence.ts (replaced by generic buildHookOutputs). The credentials reset error message is generalized.

Changes

Dynamic Channel Metadata and Hardcoded List Removal

Layer / File(s) Summary
OpenClaw runtime type definitions
src/lib/messaging/manifest/types.ts
ChannelManifest.runtime now uses ChannelRuntimeByAgentSpec, which formalizes an optional openclaw override via new ChannelOpenClawRuntimeSpec type that extends ChannelRuntimeSpec with an optional channelName field.
Channel manifest channelName fields and test helper
src/lib/messaging/channels/discord/manifest.ts, src/lib/messaging/channels/slack/manifest.ts, src/lib/messaging/channels/telegram/manifest.ts, src/lib/messaging/channels/wechat/manifest.ts, src/lib/messaging/channels/whatsapp/manifest.ts, src/lib/messaging/channels/manifests.test.ts
All five supported channels explicitly declare their OpenClaw channelName: "discord", "slack", "telegram", "openclaw-weixin", "whatsapp". Test helper expectOpenClawRuntimeVisibility updated to assert channelName alongside visibility.
New metadata helper functions
src/lib/messaging/channels/metadata.ts
Adds listMessagingChannelsWithoutCredentials (filters manifests with empty credentials arrays) and listOpenClawManagedChannelNames (extracts and de-duplicates runtime.openclaw.channelName values from OpenClaw-supported manifests).
Metadata helper test coverage
src/lib/messaging/channels/metadata.test.ts
Tests validate that listMessagingChannelsWithoutCredentials() returns ["whatsapp"] by default and filters correctly by credentials array; listOpenClawManagedChannelNames() returns the default managed list and correctly derives names from JSON-fragment render paths and explicit manifest metadata.
Dynamic managed-channel allowlist in config merge
src/lib/state/openclaw-config-merge.ts, src/lib/state/openclaw-config-merge.test.ts
Replaces the hardcoded managedChannels array in OPENCLAW_CONFIG_RESTORE_OWNERSHIP with MANAGED_OPENCLAW_CHANNEL_NAMES derived from listOpenClawManagedChannelNames(). Test extended to cover whatsapp resurrection prevention alongside telegram and openclaw-weixin.
Dynamic credentialless channel set in deployment verification
src/lib/verify-deployment.ts
Replaces the hardcoded tokenless channel constant with CREDENTIALLESS_MESSAGING_CHANNELS from listMessagingChannelsWithoutCredentials() and updates the bridge verification skip condition to use the dynamic set.
Generic hook output execution in persistence
src/lib/messaging/persistence.ts
Removes the WeChat-specific buildWechatSeedOpenClawAccountOutputs special case. Introduces buildHookOutputs helper that runs any hook generically via runMessagingHookSync with BUILT_IN_MESSAGING_HOOK_REGISTRY, filtering inputs via selectHookInputs.
Generalized channel placeholder in CLI error output
src/commands/credentials/reset.ts, test/credentials-cli-command.test.ts
Updates the credentials reset bridge-provider failure message to recommend generic channels remove <channel> instead of listing specific channel names. CLI test asserts the new generic text is present and the old discord-specific variant is absent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • NVIDIA/NemoClaw#3896: This PR directly implements core components of the manifest-first messaging architecture: adding channelName fields to channel manifests, creating helper functions to derive managed channel metadata from manifests, updating type definitions to support OpenClaw-specific runtime specs, and transitioning from channel-specific handlers to generic hook execution paths.

Suggested labels

v0.0.66

Suggested reviewers

  • cv

Poem

🐇 No more lists carved in stone,
The channels now speak on their own!
From fragments and paths we read,
Each manifest plants the seed —
Hardcoded arrays, begone and gone! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% 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 accurately summarizes the main refactoring objective of removing hardcoded messaging channel references across the codebase.
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 refactor/messaging-channel-manifest-cleanup

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

@github-code-quality

github-code-quality Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the refactor/messaging-c... 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 refactor/messaging-c... 6b76e17 +/-
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 refactor/messaging-c... branch is 44%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/messaging-c... 6b76e17 +/-
src/lib/state/o...oard-session.ts — 90% —
src/lib/inference/local.ts — 78% —
src/lib/sandbox/config.ts — 72% —
src/lib/inference/nim.ts — 72% —
src/lib/onboard/preflight.ts — 64% —
src/lib/state/sandbox.ts — 55% —
src/lib/actions...licy-channel.ts — 52% —
src/lib/onboard...er-gpu-patch.ts — 50% —
src/lib/policy/index.ts — 49% —
src/lib/onboard.ts — 17% —

Updated June 15, 2026 18:02 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • Restore still resurrects legacy WeChat channel blocks (src/lib/state/openclaw-config-merge.test.ts:78): The PR fixes the prior hook-owned WeChat gap for `channels.openclaw-weixin` by adding explicit `runtime.openclaw.channelName`, but the restore regression now expects backup `channels.wechat` to survive when the fresh rebuilt config omits channels. That leaves a built-in WeChat-shaped OpenClaw channel block outside `MANAGED_OPENCLAW_CHANNELS`, so stale account state, placeholder revisions, or enablement can be restored even though the rebuild omitted it. Nearby code still treats `wechat` as a built-in/runtime channel name in several surfaces, so this is not proven to be durable user-owned custom config.
    • Recommendation: Either include the legacy `wechat` OpenClaw channel key in managed restore ownership until it is impossible for supported runtimes to consume it, or add a documented migration/source-of-truth proof that `channels.wechat` is obsolete user-owned data. Add an all-built-ins restore regression covering `telegram`, `discord`, `slack`, `whatsapp`, `wechat`, and `openclaw-weixin` omitted from fresh config, while preserving a custom channel such as `matrix`.
    • Evidence: `OPENCLAW_CONFIG_RESTORE_OWNERSHIP.managedChannels` is now `listOpenClawManagedChannelNames()`, and WeChat declares only `channelName: "openclaw-weixin"`. In the changed test, backup contains both `wechat` and `openclaw-weixin`; the assertion preserves `channels.wechat` while asserting `channels["openclaw-weixin"]` is undefined.

🔎 Worth checking

  • Source-of-truth review needed: Managed OpenClaw channel restore ownership: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `listOpenClawManagedChannelNames()` derives only `openclaw-weixin` for WeChat, while the changed restore test preserves backup `channels.wechat`.
  • Source-of-truth review needed: Compact persisted messaging plan hook-output hydration: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `hydrateDerivedSandboxMessagingPlanFields()` fills empty `buildSteps` via `buildStepsFromManifests()`, and `buildHookOutputs()` now delegates to `runMessagingHookSync()`.
  • Source-of-truth review needed: Credentialless messaging provider checks: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `listMessagingChannelsWithoutCredentials()` filters by empty `credentials`, and `verifyDeployment()` skips missing-provider checks for channels in that derived set.
  • Compact WeChat hook-output hydration is not pinned at the public parse boundary (src/lib/messaging/persistence.ts:540): Compact persisted plans intentionally omit derived `buildSteps`, and this PR now reconstructs build-file outputs by invoking `runMessagingHookSync()` through the built-in hook registry. That removes the WeChat-specific special case, but also changes the failure behavior from swallowed WeChat seed exceptions to propagated hook-runner exceptions. Existing compact-plan coverage exercises Telegram only; it does not prove valid WeChat compact hydration reconstructs the expected build files, or that missing/unsafe account ids fail in the intended way when callers use the public parse/hydrate path.
    • Recommendation: Add focused public-boundary tests that parse or hydrate a compact active OpenClaw WeChat plan with `wechatConfig.accountId`, `wechatConfig.baseUrl`, `wechatConfig.userId`, and the credential placeholder, then assert valued build steps for `openclaw-weixin/accounts.json`, `openclaw-weixin/accounts/<accountId>.json`, and the `openclaw.json` patch. Add missing and unsafe `wechatConfig.accountId` cases and assert the intended null/throw fail-closed behavior.
    • Evidence: `compactSandboxMessagingPlanForPersistence()` strips `buildSteps`; `hydrateDerivedSandboxMessagingPlanFields()` rebuilds them with `buildStepsFromManifests()`; `buildHookOutputs()` now returns `runMessagingHookSync(...).outputs`. Existing compact persisted-plan tests in `src/lib/messaging/plan-validation.test.ts` cover Telegram, while WeChat build-output tests cover normal compiler/hook paths.
  • Generic hook-output hydration needs an explicit pure-hook contract (src/lib/messaging/persistence.ts:540): During persisted-plan hydration, `hookBuildSteps()` filters hooks by build-output declarations and then executes the registered built-in hook handler synchronously. Today this appears to reach only `wechat.seedOpenClawAccount`, which is synchronous and validates filename/URL inputs. However, the boundary is now generic: a future built-in hook with build outputs could perform host, network, or other side effects during parse/hydration unless the contract limits these handlers to pure output synthesis.
    • Recommendation: Constrain this path to explicitly pure build-output hooks, or add manifest/registration metadata that distinguishes pure reconstruction hooks from operational hooks. Add a regression that a non-pure or async build-output hook cannot be invoked from compact-plan hydration.
    • Evidence: `hookBuildSteps()` selects outputs with kind `build-arg`, `build-file`, or `package-install`, then `buildHookOutputs()` calls `runMessagingHookSync(hook, BUILT_IN_MESSAGING_HOOK_REGISTRY, ...)`. The sync runner rejects promises, but does not otherwise enforce side-effect freedom.
  • Credentialless provider skipping is still inferred from empty credential metadata (src/lib/verify-deployment.ts:136): `verifyDeployment()` now derives the provider-check skip set from manifests whose `credentials.length === 0`. That keeps WhatsApp healthy without a gateway provider, but it still couples two different concepts: no credential material and no provider/service check required. A future credentialless channel that still needs a runtime/provider health check could be reported healthy with its bridge missing.
    • Recommendation: Make provider-check requirements explicit in channel runtime metadata, such as `providerCheck: "none"` for WhatsApp or `requiresGatewayProviderCheck: false`. Add regressions showing WhatsApp remains providerless healthy, a credentialed channel such as Telegram still warns when its provider is missing, and an unknown/non-credentialless configured channel falls back to a missing-provider warning.
    • Evidence: `CREDENTIALLESS_MESSAGING_CHANNELS` is initialized from `listMessagingChannelsWithoutCredentials()`, whose implementation filters manifests by `manifest.credentials.length === 0`. Existing verify-deployment tests cover WhatsApp tokenless behavior and provider checks for known credentialed channels, but the source of truth remains implicit.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — Restore backup channels for `telegram`, `discord`, `slack`, `whatsapp`, `wechat`, and `openclaw-weixin` while fresh config omits all channels; assert built-in/runtime-owned keys are not resurrected and custom `matrix` is preserved.. The PR changes runtime/sandbox-adjacent restore ownership, deployment verification, and persisted messaging plan reconstruction. Unit tests help, but the risky behavior is at public restore/parse/verification boundaries.
  • **Runtime validation** — If preserving `channels.wechat` is intentional, prove with a regression or fixture that supported OpenClaw runtimes no longer consume it, and document the compatibility removal condition.. The PR changes runtime/sandbox-adjacent restore ownership, deployment verification, and persisted messaging plan reconstruction. Unit tests help, but the risky behavior is at public restore/parse/verification boundaries.
  • **Runtime validation** — Parse or hydrate a compact active OpenClaw WeChat plan with `wechatConfig.accountId`, `wechatConfig.baseUrl`, `wechatConfig.userId`, and credential placeholder; assert build-file steps for accounts index, per-account file, and `openclaw.json` patch.. The PR changes runtime/sandbox-adjacent restore ownership, deployment verification, and persisted messaging plan reconstruction. Unit tests help, but the risky behavior is at public restore/parse/verification boundaries.
  • **Runtime validation** — Parse or hydrate compact active WeChat plans with missing and unsafe `wechatConfig.accountId`; assert the intended fail-closed behavior at the public parser/hydration boundary.. The PR changes runtime/sandbox-adjacent restore ownership, deployment verification, and persisted messaging plan reconstruction. Unit tests help, but the risky behavior is at public restore/parse/verification boundaries.
  • **Runtime validation** — Run verify-deployment for an unknown configured channel with no provider metadata and `providerExistsInGateway` false; assert a messaging warning rather than a credentialless skip.. The PR changes runtime/sandbox-adjacent restore ownership, deployment verification, and persisted messaging plan reconstruction. Unit tests help, but the risky behavior is at public restore/parse/verification boundaries.
  • **Compact WeChat hook-output hydration is not pinned at the public parse boundary** — Add focused public-boundary tests that parse or hydrate a compact active OpenClaw WeChat plan with `wechatConfig.accountId`, `wechatConfig.baseUrl`, `wechatConfig.userId`, and the credential placeholder, then assert valued build steps for `openclaw-weixin/accounts.json`, `openclaw-weixin/accounts/<accountId>.json`, and the `openclaw.json` patch. Add missing and unsafe `wechatConfig.accountId` cases and assert the intended null/throw fail-closed behavior.
  • **Managed OpenClaw channel restore ownership** — Partial: the test covers `telegram`, `whatsapp`, `openclaw-weixin`, and custom `matrix`, but omits `discord` and `slack` in the restore-omission case and expects legacy `wechat` to be restored.. `listOpenClawManagedChannelNames()` derives only `openclaw-weixin` for WeChat, while the changed restore test preserves backup `channels.wechat`.
  • **Compact persisted messaging plan hook-output hydration** — Missing for WeChat at the public compact parse/hydrate boundary, including valid reconstruction and missing/unsafe account-id failure behavior.. `hydrateDerivedSandboxMessagingPlanFields()` fills empty `buildSteps` via `buildStepsFromManifests()`, and `buildHookOutputs()` now delegates to `runMessagingHookSync()`.
Since last review details

Current findings:

  • Source-of-truth review needed: Managed OpenClaw channel restore ownership: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `listOpenClawManagedChannelNames()` derives only `openclaw-weixin` for WeChat, while the changed restore test preserves backup `channels.wechat`.
  • Source-of-truth review needed: Compact persisted messaging plan hook-output hydration: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `hydrateDerivedSandboxMessagingPlanFields()` fills empty `buildSteps` via `buildStepsFromManifests()`, and `buildHookOutputs()` now delegates to `runMessagingHookSync()`.
  • Source-of-truth review needed: Credentialless messaging provider checks: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `listMessagingChannelsWithoutCredentials()` filters by empty `credentials`, and `verifyDeployment()` skips missing-provider checks for channels in that derived set.
  • Restore still resurrects legacy WeChat channel blocks (src/lib/state/openclaw-config-merge.test.ts:78): The PR fixes the prior hook-owned WeChat gap for `channels.openclaw-weixin` by adding explicit `runtime.openclaw.channelName`, but the restore regression now expects backup `channels.wechat` to survive when the fresh rebuilt config omits channels. That leaves a built-in WeChat-shaped OpenClaw channel block outside `MANAGED_OPENCLAW_CHANNELS`, so stale account state, placeholder revisions, or enablement can be restored even though the rebuild omitted it. Nearby code still treats `wechat` as a built-in/runtime channel name in several surfaces, so this is not proven to be durable user-owned custom config.
    • Recommendation: Either include the legacy `wechat` OpenClaw channel key in managed restore ownership until it is impossible for supported runtimes to consume it, or add a documented migration/source-of-truth proof that `channels.wechat` is obsolete user-owned data. Add an all-built-ins restore regression covering `telegram`, `discord`, `slack`, `whatsapp`, `wechat`, and `openclaw-weixin` omitted from fresh config, while preserving a custom channel such as `matrix`.
    • Evidence: `OPENCLAW_CONFIG_RESTORE_OWNERSHIP.managedChannels` is now `listOpenClawManagedChannelNames()`, and WeChat declares only `channelName: "openclaw-weixin"`. In the changed test, backup contains both `wechat` and `openclaw-weixin`; the assertion preserves `channels.wechat` while asserting `channels["openclaw-weixin"]` is undefined.
  • Compact WeChat hook-output hydration is not pinned at the public parse boundary (src/lib/messaging/persistence.ts:540): Compact persisted plans intentionally omit derived `buildSteps`, and this PR now reconstructs build-file outputs by invoking `runMessagingHookSync()` through the built-in hook registry. That removes the WeChat-specific special case, but also changes the failure behavior from swallowed WeChat seed exceptions to propagated hook-runner exceptions. Existing compact-plan coverage exercises Telegram only; it does not prove valid WeChat compact hydration reconstructs the expected build files, or that missing/unsafe account ids fail in the intended way when callers use the public parse/hydrate path.
    • Recommendation: Add focused public-boundary tests that parse or hydrate a compact active OpenClaw WeChat plan with `wechatConfig.accountId`, `wechatConfig.baseUrl`, `wechatConfig.userId`, and the credential placeholder, then assert valued build steps for `openclaw-weixin/accounts.json`, `openclaw-weixin/accounts/<accountId>.json`, and the `openclaw.json` patch. Add missing and unsafe `wechatConfig.accountId` cases and assert the intended null/throw fail-closed behavior.
    • Evidence: `compactSandboxMessagingPlanForPersistence()` strips `buildSteps`; `hydrateDerivedSandboxMessagingPlanFields()` rebuilds them with `buildStepsFromManifests()`; `buildHookOutputs()` now returns `runMessagingHookSync(...).outputs`. Existing compact persisted-plan tests in `src/lib/messaging/plan-validation.test.ts` cover Telegram, while WeChat build-output tests cover normal compiler/hook paths.
  • Generic hook-output hydration needs an explicit pure-hook contract (src/lib/messaging/persistence.ts:540): During persisted-plan hydration, `hookBuildSteps()` filters hooks by build-output declarations and then executes the registered built-in hook handler synchronously. Today this appears to reach only `wechat.seedOpenClawAccount`, which is synchronous and validates filename/URL inputs. However, the boundary is now generic: a future built-in hook with build outputs could perform host, network, or other side effects during parse/hydration unless the contract limits these handlers to pure output synthesis.
    • Recommendation: Constrain this path to explicitly pure build-output hooks, or add manifest/registration metadata that distinguishes pure reconstruction hooks from operational hooks. Add a regression that a non-pure or async build-output hook cannot be invoked from compact-plan hydration.
    • Evidence: `hookBuildSteps()` selects outputs with kind `build-arg`, `build-file`, or `package-install`, then `buildHookOutputs()` calls `runMessagingHookSync(hook, BUILT_IN_MESSAGING_HOOK_REGISTRY, ...)`. The sync runner rejects promises, but does not otherwise enforce side-effect freedom.
  • Credentialless provider skipping is still inferred from empty credential metadata (src/lib/verify-deployment.ts:136): `verifyDeployment()` now derives the provider-check skip set from manifests whose `credentials.length === 0`. That keeps WhatsApp healthy without a gateway provider, but it still couples two different concepts: no credential material and no provider/service check required. A future credentialless channel that still needs a runtime/provider health check could be reported healthy with its bridge missing.
    • Recommendation: Make provider-check requirements explicit in channel runtime metadata, such as `providerCheck: "none"` for WhatsApp or `requiresGatewayProviderCheck: false`. Add regressions showing WhatsApp remains providerless healthy, a credentialed channel such as Telegram still warns when its provider is missing, and an unknown/non-credentialless configured channel falls back to a missing-provider warning.
    • Evidence: `CREDENTIALLESS_MESSAGING_CHANNELS` is initialized from `listMessagingChannelsWithoutCredentials()`, whose implementation filters manifests by `manifest.credentials.length === 0`. Existing verify-deployment tests cover WhatsApp tokenless behavior and provider checks for known credentialed channels, but the source of truth remains implicit.

Workflow run details

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

@github-actions

github-actions Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: messaging-providers-e2e, channels-stop-start-openclaw-e2e
Optional E2E: channels-stop-start-hermes-e2e, rebuild-openclaw-e2e, credential-migration-e2e

Dispatch hint: messaging-providers-e2e,channels-stop-start-openclaw-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • messaging-providers-e2e (high): Exercises the full messaging provider/placeholder/L7-proxy chain for Telegram, Discord, Slack, and WeChat, plus WhatsApp's credentialless no-provider path. This directly covers the changed manifest runtime metadata, credentialless-channel metadata used by deployment verification, and generic hook-output execution in messaging persistence.
  • channels-stop-start-openclaw-e2e (high): Covers OpenClaw stop/start across rebuild for all built-in channels, including WeChat's openclaw-weixin runtime key and WhatsApp. This is the closest existing coverage for manifest-derived managed channel names and for ensuring current channel state is not lost or resurrected during rebuild.

Optional E2E

  • channels-stop-start-hermes-e2e (high): Useful adjacent confidence because the manifest and persistence changes are shared across OpenClaw and Hermes channel planning, though the most risky restored openclaw.json channelName change is OpenClaw-specific.
  • rebuild-openclaw-e2e (high): Additional confidence for the OpenClaw rebuild/config-restore path touched by openclaw-config-merge.ts, including config-hash and sanitized backup behavior. It is optional because it does not specifically exercise managed messaging channel restore semantics.
  • credential-migration-e2e (medium): Adjacent credential-flow confidence for changes near credential reset and messaging provider filtering, but the PR's credential command change is primarily guidance text and is covered by unit tests.

New E2E recommendations

  • OpenClaw config restore with managed messaging channels (high): Existing rebuild/restore E2Es do not appear to deliberately seed a backed-up openclaw.json containing stale managed channels such as telegram, whatsapp, and openclaw-weixin, rebuild with those channels absent, and assert they are not resurrected while custom channels remain.
    • Suggested test: Add an OpenClaw rebuild/restore E2E that creates a stale openclaw.json backup with managed and custom channel entries, rebuilds from current generated config, and verifies manifest-derived managed channels are owned by NemoClaw while backup-only custom channels are preserved.
  • future credentialless messaging channels (medium): WhatsApp is covered as the current credentialless channel, but there is no dedicated E2E proving that deployment verification and bridge health automatically handle any new manifest-declared credentialless channel without adding hard-coded lists.
    • Suggested test: Add a lightweight hermetic E2E or scenario fixture with a synthetic credentialless channel manifest and verify deployment bridge checks skip provider existence only for manifest-declared credentialless channels.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: messaging-providers-e2e,channels-stop-start-openclaw-e2e

@github-actions

github-actions Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: messaging-providers-vitest, channels-add-remove-vitest, state-backup-restore-vitest
Optional Vitest E2E scenarios: messaging-compatible-endpoint-vitest

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=messaging-providers-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=channels-add-remove-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=state-backup-restore-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • messaging-providers-vitest: The PR changes built-in messaging channel manifests, manifest runtime metadata, metadata helpers, credentialless-channel handling, persistence hook output plumbing, and deployment verification. The free-standing messaging providers Vitest job is the focused live coverage for all built-in messaging providers, including Telegram/Discord/Slack/WeChat provider placeholders, WhatsApp QR-only no-provider behavior, OpenClaw channel/plugin config, and policy/runtime checks.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=messaging-providers-vitest
  • channels-add-remove-vitest: The PR changes channel manifest runtime ownership metadata and managed-channel restore behavior used when channels are added, rebuilt, removed, and rendered into openclaw.json. The channels add/remove live Vitest job exercises the real CLI/OpenShell boundary for channel lifecycle, rebuild, provider registration/removal, policy application, and in-sandbox rendered state.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=channels-add-remove-vitest
  • state-backup-restore-vitest: The PR changes src/lib/state/openclaw-config-merge.ts so OpenClaw managed channel ownership is derived from manifest runtime metadata. The state backup/restore live Vitest job is the focused workflow-dispatchable coverage for the backup/restore state surface that consumes this merge behavior during real sandbox restore flows.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=state-backup-restore-vitest

Optional Vitest E2E scenarios

  • messaging-compatible-endpoint-vitest: Adjacent coverage for the Telegram/OpenClaw messaging path with a compatible inference endpoint. Useful if maintainers want extra confidence that the manifest/runtime metadata refactor did not regress messaging plus inference routing, but messaging-providers-vitest is the primary broad messaging surface for this PR.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=messaging-compatible-endpoint-vitest

Relevant changed files

  • src/commands/credentials/reset.ts
  • src/lib/messaging/channels/discord/manifest.ts
  • src/lib/messaging/channels/metadata.ts
  • src/lib/messaging/channels/slack/manifest.ts
  • src/lib/messaging/channels/telegram/manifest.ts
  • src/lib/messaging/channels/wechat/manifest.ts
  • src/lib/messaging/channels/whatsapp/manifest.ts
  • src/lib/messaging/manifest/types.ts
  • src/lib/messaging/persistence.ts
  • src/lib/state/openclaw-config-merge.ts
  • src/lib/verify-deployment.ts

@sandl99 sandl99 changed the title refactor(messaging): clean up some hard code stragglers refactor(messaging): clean up some hard code stragglers Jun 15, 2026
@sandl99 sandl99 added refactor PR restructures code without intended behavior change area: messaging Messaging channels, bridges, manifests, or channel lifecycle labels Jun 15, 2026
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@sandl99
sandl99 marked this pull request as ready for review June 15, 2026 16:21
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@sandl99
sandl99 requested a review from cv June 15, 2026 17:20
@sandl99
sandl99 marked this pull request as draft June 15, 2026 17:36
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@sandl99
sandl99 marked this pull request as ready for review June 15, 2026 17:59
@sandl99 sandl99 removed the v0.0.65 label Jun 15, 2026
@sandl99

sandl99 commented Jun 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • Restore still resurrects legacy WeChat channel blocks (src/lib/state/openclaw-config-merge.test.ts:78): The PR fixes the prior hook-owned WeChat gap for channels.openclaw-weixin by adding explicit runtime.openclaw.channelName, but the restore regression now expects backup channels.wechat to survive when the fresh rebuilt config omits channels. That leaves a built-in WeChat-shaped OpenClaw channel block outside MANAGED_OPENCLAW_CHANNELS, so stale account state, placeholder revisions, or enablement can be restored even though the rebuild omitted it. Nearby code still treats wechat as a built-in/runtime channel name in several surfaces, so this is not proven to be durable user-owned custom config.

Thanks for the careful read. I do not think channels.wechat is a legacy OpenClaw runtime channel key we need to preserve or manage. The supported OpenClaw WeChat key is channels.openclaw- weixin; the other wechat references are legacy and redundant config

@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27565832939
Target ref: refactor/messaging-channel-manifest-cleanup
Requested jobs: messaging-providers-e2e,messaging-compatible-endpoint-e2e,channels-add-remove-e2e,channels-stop-start-openclaw-e2e,channels-stop-start-hermes-e2e,openclaw-slack-pairing-e2e,openclaw-discord-pairing-e2e,hermes-slack-e2e,hermes-discord-e2e,credential-migration-e2e,rebuild-openclaw-e2e,rebuild-hermes-e2e,state-backup-restore-e2e,network-policy-e2e
Summary: 14 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
channels-add-remove-e2e ✅ success
channels-stop-start-hermes-e2e ✅ success
channels-stop-start-openclaw-e2e ✅ success
credential-migration-e2e ✅ success
hermes-discord-e2e ✅ success
hermes-slack-e2e ✅ success
messaging-compatible-endpoint-e2e ✅ success
messaging-providers-e2e ✅ success
network-policy-e2e ✅ success
openclaw-discord-pairing-e2e ✅ success
openclaw-slack-pairing-e2e ✅ success
rebuild-hermes-e2e ✅ success
rebuild-openclaw-e2e ✅ success
state-backup-restore-e2e ✅ success

@sandl99

sandl99 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

It's ready to be merged now @cv Thanks.

@cv
cv merged commit 999a6a8 into main Jun 15, 2026
189 of 192 checks passed
@cv
cv deleted the refactor/messaging-channel-manifest-cleanup branch June 15, 2026 18:47
@miyoungc miyoungc mentioned this pull request Jun 16, 2026
5 of 13 tasks
cv pushed a commit that referenced this pull request Jun 17, 2026
## Summary
Refreshes release-prep documentation for NemoClaw v0.0.65.
Adds the v0.0.65 release-notes section and refreshes generated
`nemoclaw-user-*` skills from the Fern MDX source docs.

## Changes
- Added the v0.0.65 release notes to `docs/about/release-notes.mdx` with
links to the deeper docs pages for lifecycle, troubleshooting,
inference, CLI commands, messaging, credentials, network policy, Hermes,
and sub-agents.
- Regenerated the `nemoclaw-user-*` skills with
`scripts/docs-to-skills.py` so release-prep skill output matches the
merged source docs.
- Used the v0.0.65 announcement discussion as release context:
#5472.

## Source Summary
- #2492 -> `docs/about/release-notes.mdx`: Documents deadline-based
gateway wait reliability in the v0.0.65 recovery summary.
- #4958 -> `docs/about/release-notes.mdx`: Documents re-execed OpenClaw
gateway health check recovery in the sandbox recovery summary.
- #5163 -> `docs/about/release-notes.mdx`: Documents safer uninstall TTY
confirmation behavior in the day-two CLI summary.
- #5178 -> `docs/about/release-notes.mdx`: Documents fail-closed config
restore merge behavior in the rebuild and restore summary.
- #5179 -> `docs/about/release-notes.mdx`: Documents WeChat QR token
redaction in the messaging summary.
- #5182 -> `docs/about/release-notes.mdx`: Documents sustained gateway
serving checks in the recovery summary.
- #5194 -> `docs/about/release-notes.mdx`: Documents model-router
teardown during uninstall in the day-two CLI summary.
- #5195 -> `docs/about/release-notes.mdx`: Documents Shields
auto-restore lock reconfirmation in the rebuild and restore summary.
- #5198 -> `docs/about/release-notes.mdx`: Documents Docker Desktop WSL
CDI injection failure handling in the onboarding diagnostics summary.
- #5201 -> `docs/about/release-notes.mdx`: Documents sandbox
download/upload wrappers and sessions export in the day-two CLI summary.
- #5205 -> `docs/about/release-notes.mdx`: Documents reporter-owned
model metadata preservation in the rebuild and restore summary.
- #5214 -> `docs/about/release-notes.mdx`: Documents managed vLLM model
preflight before side effects in the inference setup summary.
- #5215 -> `docs/about/release-notes.mdx`: Documents managed vLLM extra
serve arguments in the inference setup summary.
- #5216 -> `docs/about/release-notes.mdx`: Documents silent OpenClaw
runtime fallback surfacing in the onboarding diagnostics summary.
- #5225 -> `docs/about/release-notes.mdx`: Documents persisted sandbox
gateway lookup in the gateway recovery summary.
- #5238 -> `docs/about/release-notes.mdx`: Documents sub-agent gateway
dial-back through the sandbox interface in the Hermes and sub-agent
summary.
- #5248 -> `docs/about/release-notes.mdx`: Documents Discord per-account
proxy resolution in the messaging summary.
- #5264 -> `docs/about/release-notes.mdx`: Documents reserved Hermes
port `8642` handling in the Hermes compatibility summary.
- #5267 -> `docs/about/release-notes.mdx`: Documents the narrower Hermes
baseline policy in the Hermes compatibility summary.
- #5321 -> `docs/about/release-notes.mdx`: Documents restored gateway
guard chains in the gateway recovery summary.
- #5328 -> `docs/about/release-notes.mdx`: Documents compact persisted
messaging plans in the messaging summary.
- #5338 -> `docs/about/release-notes.mdx`: Documents manifest channel
migration in the messaging summary.
- #5352 -> `docs/about/release-notes.mdx`: Documents persisted agent
preservation through registry recovery in the rebuild and restore
summary.
- #5371 ->
`.agents/skills/nemoclaw-user-reference/references/commands.md`:
Refreshes generated skill output for custom build cache and
layer-ordering source docs.
- #5379 -> `docs/about/release-notes.mdx`: Documents dashboard port
allocation across multiple NemoClaw gateways in the recovery summary.
- #5382 -> `docs/about/release-notes.mdx`: Documents recovery when an
active gateway has no sandbox spec in the recovery summary.
- #5389 ->
`.agents/skills/nemoclaw-user-reference/references/troubleshooting.md`:
Refreshes generated skill output for declared agent `forward_ports`
recovery source docs.
- #5400 -> `docs/about/release-notes.mdx`: Documents bounded compatible
endpoint probes in the inference setup summary.
- #5410 -> `docs/about/release-notes.mdx`: Documents provider credential
hash removal from sandbox registry entries in the messaging summary.
- #5418 -> `docs/about/release-notes.mdx`: Documents summarized
inference validation failures in the onboarding diagnostics summary.
- #5457 -> `docs/about/release-notes.mdx`: Documents context-window
recomputation after runtime model switches in the inference setup
summary.
- #5463 -> `docs/about/release-notes.mdx`: Documents cleanup of
hard-coded messaging channel stragglers in the messaging summary.

## Skipped
- #5366 matched `docs/.docs-skip` entries through skipped experimental
paths, so this PR does not add new release-note text for that commit.

## 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] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [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)

Verification notes:
- `npm run docs` passed after rerunning outside the sandbox. Fern
reported 0 errors and 1 hidden warning.
- The first sandboxed `npm run docs` attempt failed before validation
because `tsx` could not create its local IPC pipe under sandbox
restrictions.
- `npm run build:cli` passed before push to refresh the local `dist/`
artifacts used by the CLI typecheck hook.
- `npm test` was not run because this is a docs-only release refresh.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Released NemoClaw v0.0.65 with improved gateway/sandbox recovery,
safer day-two workflows, and enhanced Hermes compatibility.
* Added managed vLLM extra-arguments configuration via
`NEMOCLAW_VLLM_EXTRA_ARGS_JSON`.
* Added Hermes troubleshooting guidance for port forwarding and health
checks.

* **Documentation**
* Updated NVIDIA Endpoints/NIM setup and examples to use
`NVIDIA_INFERENCE_API_KEY`.
* Refined NVIDIA network policy and Model Router API base configuration.
* Expanded CLI/environment variable documentation (including sub-agent
gateway connectivity) and plugin build performance tips.

* **Tests**
  * Expanded Vitest-backed E2E release validation coverage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: messaging Messaging channels, bridges, manifests, or channel lifecycle refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants