refactor(messaging): clean up some hard code stragglers by sandl99 · Pull Request #5463 · NVIDIA/NemoClaw

sandl99 · 2026-06-15T11:49:01Z

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.

…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

copy-pr-bot · 2026-06-15T11:49:04Z

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 · 2026-06-15T11:49:09Z

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 · 2026-06-15T11:49:42Z

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 · 2026-06-15T11:53:24Z

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 · 2026-06-15T11:55:31Z

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 · 2026-06-15T11:55:32Z

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

…est-cleanup' into refactor/messaging-channel-manifest-cleanup

sandl99 · 2026-06-15T18:24:46Z

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

github-actions · 2026-06-15T18:42:33Z

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 · 2026-06-15T18:42:42Z

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

sandl99 added 23 commits June 12, 2026 19:05

refactor(messaging): migrate channel lifecycle hooks

b5f067a

refactor(messaging): finish manifest channel migration

b99378a

fix(messaging): address manifest hook review feedback

067ad60

refactor(messaging): simplify channel hook manifests

f761d0b

refactor(messaging): colocate wechat host qr login

0de062b

fix(messaging): address metadata review comments

59cf532

refactor(messaging): compact persisted plan state

5c71c1f

Merge remote-tracking branch 'origin/main' into refactor/messaging-ch…

f37ff32

…annel-manifest-stragglers # Conflicts: # ci/test-file-size-budget.json

fix(messaging): preserve persisted channel plan fields

cbe09ef

fix(messaging): bake runtime setup artifact

72ccad9

test(e2e): align WhatsApp preload guard assertion

c02fedb

Merge branch 'main' into refactor/messaging-channel-manifest-stragglers

f7c0b68

refactor(messaging): move Slack deny feedback into runtime preload

75075b0

refactor(messaging): install plugins from package plan

e497815

fix(messaging): validate plan object array entries

a465123

Merge remote-tracking branch 'origin/refactor/messaging-channel-manif…

3b46a32

…est-stragglers' into refactor/messaging-channel-manifest-stragglers

refactor(messaging): move runtime preload sources to ts

848c9ec

fix(messaging): patch Slack ESM helper imports

9fc3141

fix(messaging): type channel diagnostics preloads

f7ba9f7

fix(messaging): compile runtime preloads for sandbox image

c535a6e

fix(messaging): compile runtime preloads narrowly

9b85f89

refactor(messaging): finish channel manifest cleanup

754b5a2

sandl99 self-assigned this Jun 15, 2026

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

Base automatically changed from refactor/messaging-channel-manifest-stragglers to main June 15, 2026 15:28

sandl99 changed the title ~~refactor(messaging): finish channel manifest cleanup~~ refactor(messaging): clean up some hard code stragglers Jun 15, 2026

merge(main): resolve messaging manifest cleanup conflicts

df7c432

sandl99 changed the title ~~refactor(messaging): clean up some hard code stragglers~~ refactor(messaging): clean up some hard code stragglers Jun 15, 2026

sandl99 added refactor PR restructures code without intended behavior change area: messaging Messaging channels, bridges, manifests, or channel lifecycle labels Jun 15, 2026

Merge branch 'main' into refactor/messaging-channel-manifest-cleanup

2a79a4c

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

sandl99 marked this pull request as ready for review June 15, 2026 16:21

sandl99 added the v0.0.65 Release target label Jun 15, 2026

sandl99 added 2 commits June 16, 2026 00:00

refactor(messaging): drop legacy OpenClaw channel restore list

42e149a

Merge remote-tracking branch 'origin/refactor/messaging-channel-manif…

9305d09

…est-cleanup' into refactor/messaging-channel-manifest-cleanup

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

sandl99 requested a review from cv June 15, 2026 17:20

cv approved these changes Jun 15, 2026

View reviewed changes

sandl99 marked this pull request as draft June 15, 2026 17:36

fix(messaging): declare OpenClaw channel ownership in manifests

6b76e17

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

sandl99 marked this pull request as ready for review June 15, 2026 17:59

sandl99 removed the v0.0.65 Release target label Jun 15, 2026

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

sandl99 added the v0.0.65 Release target label Jun 15, 2026

cv merged commit 999a6a8 into main Jun 15, 2026
189 of 192 checks passed

cv deleted the refactor/messaging-channel-manifest-cleanup branch June 15, 2026 18:47

Conversation

sandl99 commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Jun 15, 2026

Uh oh!

coderabbitai Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

github-code-quality Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

github-actions Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor

🛠️ Needs attention

🔎 Worth checking

🌱 Nice ideas

Uh oh!

github-actions Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Advisor Recommendation

E2E Recommendation Advisor

Required E2E

Optional E2E

New E2E recommendations

Dispatch hint

Uh oh!

github-actions Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Vitest E2E Scenario Recommendation

Vitest E2E Scenario Advisor

Required Vitest E2E scenarios

Optional Vitest E2E scenarios

Relevant changed files

Uh oh!

sandl99 commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor

🛠️ Needs attention

Uh oh!

github-actions Bot commented Jun 15, 2026

Selective E2E Results — ✅ All requested jobs passed

Uh oh!

sandl99 commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sandl99 commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

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

edited

Loading

github-actions Bot commented Jun 15, 2026 •

edited

Loading

github-actions Bot commented Jun 15, 2026 •

edited

Loading

github-actions Bot commented Jun 15, 2026 •

edited

Loading

sandl99 commented Jun 15, 2026 •

edited

Loading