Skip to content

feat(messaging): add WeCom channel support - #5666

Closed
sandl99 wants to merge 10 commits into
mainfrom
feat/wecom-messaging
Closed

feat(messaging): add WeCom channel support#5666
sandl99 wants to merge 10 commits into
mainfrom
feat/wecom-messaging

Conversation

@sandl99

@sandl99 sandl99 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds WeCom as an experimental manifest-first messaging channel for OpenClaw and Hermes, including credential rendering, policy presets, OpenClaw plugin installation metadata, and Hermes config output. Also emits websocket proxy environment variables from the existing OpenShell proxy URL so aiohttp can route WeCom wss:// connections through the L7 proxy.

Result

image

Related Issue

Refs #5492

Changes

  • Add the WeCom channel manifest, template resolver, built-in registration, and channel metadata coverage.
  • Add WeCom OpenShell policy presets for OpenClaw and Hermes, including the WeCom AI Bot websocket and REST endpoints.
  • Render WeCom OpenClaw and Hermes config from manifest state, including DM policy and allowed user IDs.
  • Export WS_PROXY/WSS_PROXY alongside the existing HTTP proxy variables for runtime websocket clients.
  • Add targeted tests for WeCom manifest compilation, build rendering, onboard setup, policies, credentials, and runtime proxy env emission.

PR Review Advisor justification

PR Review Advisor warning justifications:

  • PRA-1 / PRA-4: Keep as-is. WS_PROXY/WSS_PROXY are exported from the same OpenShell proxy source as HTTP_PROXY/HTTPS_PROXY so WeCom WebSocket clients can use the enforced policy path. Existing startup tests cover env emission and NO_PROXY includes loopback/gateway bypass entries; deeper route behavior belongs in runtime/E2E validation, not this manifest wiring PR.
  • PRA-2 / PRA-3: Keep as-is. WeCom follows the current OpenClaw messaging default: DM access is open unless the operator configures an allowlist/policy. The PR already exposes WECOM_DM_POLICY and WECOM_ALLOWED_USERS so operators can restrict access without changing the default behavior.
  • PRA-5: Justified. GET /cgi-bin/gettoken was removed from both policy surfaces, so the query-secret Agent token exchange is not allowed by this PR. The remaining REST/WebSocket policy covers the AI Bot paths used by the rendered Bot ID/Secret configuration.
  • PRA-6: Justified. The OpenClaw plugin package is exact-version pinned as npm:@wecom/wecom-openclaw-plugin@2026.5.25; package provenance/advisory metadata is not currently part of the channel manifest schema. Adding a repo-wide package trust metadata contract should be handled separately.
  • PRA-7: Deferred intentionally. Docs are not being updated in this PR yet; WeCom docs will be handled in the later docs-update window so this feature PR stays code/test/policy scoped.
  • PRA-8 / PRA-T*: Deferred. Existing tests cover manifest registration, render output, runtime visibility, policy presets, and package install planning. Further E2E/runtime route validation and test-file extraction can be handled as follow-up hardening.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • 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

Release Notes

  • New Features

    • Added WeCom (Enterprise WeChat) as a supported messaging platform, including built-in templates, credentials, and a WeCom AI Bot network/policy preset.
    • Extended platform support across agent and onboarding flows so WeCom can be enabled via configuration.
  • Bug Fixes

    • Improved channel enablement detection to recognize an enabled: true channel configuration shape (in addition to account-level enabled flags).
  • Documentation

    • Updated messaging guidance and mock/testing instructions to include WeCom.

@sandl99 sandl99 self-assigned this Jun 23, 2026
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds WeCom (Enterprise WeChat) as a new built-in messaging channel. A new channel manifest and template resolver are introduced, along with a wecom_aibot network policy covering WebSocket and REST endpoints. Agent manifests for Hermes and OpenClaw are updated, WebSocket proxy variables (WS_PROXY/WSS_PROXY) are propagated in startup scripts, runtime-visibility contracts are extended to support config-shape-driven channel detection, and tests across registry, onboarding, compiler, and build-applier layers are expanded.

Changes

WeCom Channel Integration

Layer / File(s) Summary
WeCom channel manifest and template resolver
src/lib/messaging/channels/wecom/manifest.ts, src/lib/messaging/channels/wecom/template-resolver.ts, src/lib/messaging/manifest/types.ts, src/lib/messaging/AGENTS.md
Defines and exports wecomManifest with token-paste auth mode, inputs (botId, secret, allowedUsers, dmPolicy with valid values and default), credential mappings, render steps for both OpenClaw (openclaw.json) and Hermes (env/config updates), runtime visibility with configShape: "enabled-flag", persisted state keys with hydration mappings, and enrollment hooks for token pasting and config prompts. Exports resolveWecomTemplateReference handling dmPolicy validation against fixed set and `allowedIds.wecom.values
Channel registry wiring and network policies
src/lib/messaging/channels/built-ins.ts, src/lib/messaging/channels/template-resolver.ts, agents/hermes/policy-additions.yaml, nemoclaw-blueprint/policies/presets/wecom.yaml, nemoclaw-blueprint/policies/tiers.yaml
Registers wecomManifest in BUILT_IN_CHANNEL_MANIFESTS and resolveWecomTemplateReference in BUILT_IN_TEMPLATE_REFERENCE_RESOLVERS. Adds wecom_aibot network policy in Hermes policy-additions with WebSocket endpoint to openws.work.weixin.qq.com:443 (allow GET/WEBSOCKET_TEXT all paths) and REST endpoint to qyapi.weixin.qq.com:443 (allow specific /cgi-bin/ methods), both with credential rewrite enabled; creates parallel blueprint preset YAML; updates open tier to include wecom with read-write access.
Agent manifests and WebSocket proxy propagation
agents/hermes/manifest.yaml, agents/openclaw/manifest.yaml, agents/hermes/Dockerfile.base, agents/hermes/start.sh, scripts/nemoclaw-start.sh
Adds wecom to messaging_platforms.supported for both Hermes and OpenClaw agents. Adds wecom to OpenClaw's state_dirs list. Reformats Hermes Dockerfile integration comment. Exports WS_PROXY/WSS_PROXY and lowercase ws_proxy/wss_proxy variants alongside HTTP/HTTPS proxies in both startup scripts' main runtime env and generated /tmp/*-proxy-env.sh shim files. Expands proxy comments documenting casing preferences and WS/WSS routing.
Runtime visibility contracts and channel detection
src/lib/messaging/manifest/types.ts, src/lib/messaging/channels/metadata.ts, src/lib/channel-runtime-status.ts
Updates ChannelRuntimeVisibilitySpec with optional configShape discriminator ("accounts" or "enabled-flag"). Extends OpenClawRuntimeChannelMetadata with configShape property, populated from manifest visibility or defaulting to "accounts". Refactors extractEnabledChannelsFromOpenclawConfig to map config keys to full runtime metadata and support configShape === "enabled-flag" for direct channelConfig.enabled checks alongside existing accounts iteration. Replaces runtimeConfigKeyToChannelName helper with runtimeConfigKeyToMetadata.
Manifest registry, metadata, and diagnostics tests
src/lib/messaging/channels/manifests.test.ts, src/lib/messaging/channels/metadata.test.ts, src/lib/messaging/diagnostics.test.ts, src/lib/agent/defs.test.ts, src/lib/channel-runtime-status.test.ts
Extends manifest registry phase-1 tests to expect wecom in listAvailable() for both agents. Adds wecomManifest to test imports and metadata comparison map. Extends helper to optionally validate configShape in runtime visibility. Adds comprehensive WeCom render-intent test block validating credentials, env keys, config defaults, Hermes env-lines, render JSON markers, policy structure, npm package spec/pinning, state hydration mappings, and enroll hooks including OpenClaw configShape support. Updates metadata tests for credential env key resolution, policy aliases, OpenClaw managed channel names, and package install specs. Extends agent-defs and diagnostics expectations. Adds channel-runtime-status tests for configShape: "enabled-flag" detection and WeCom log-probe recognition.
Onboarding and credential management tests
src/lib/messaging/applier/setup-applier.test.ts, src/lib/messaging-channel-config.test.ts, src/lib/onboard/extra-placeholder-keys.test.ts, src/lib/onboard/initial-policy.test.ts, src/lib/onboard/messaging-channel-setup.test.ts, src/lib/onboard/messaging-reuse.test.ts, src/lib/onboard/messaging-prep.test.ts
Adds WeCom credentials (WECOM_BOT_ID, WECOM_SECRET) to TEST_CREDENTIALS and includes comprehensive setup-applier test for WeCom provider upsert, per-call env scoping, and credential redaction. Extends config sanitization to allow WECOM_ALLOWED_USERS/WECOM_DM_POLICY env keys. Adds WeCom keys to canonical placeholder fixtures. Updates initial-policy test to include wecom_aibot filtering and adds new test for active WeCom policy retention. Adds non-interactive setup test for WeCom static credentials. Updates messaging-prep to use CHANNEL_BY_ENV_KEY map routing WECOM_* keys to wecom channel. Extends messaging-reuse to verify WeCom provider availability. Updates setup-applier policy preset test to include WeCom policy key and presets.
Sandbox channel and registry tests
src/lib/sandbox/channels.test.ts, test/credentials.test.ts, test/sandbox-provider-cleanup.test.ts, test/channels-add-preset.test.ts
Extends KNOWN_CHANNELS test to assert wecom presence, primary env key WECOM_BOT_ID, DM allowlist metadata (env key, label, help), QR-pairing expectation (false), and multi-secret token keys. Updates credential metadata tests for WECOM_SECRET. Updates channel name normalization and listChannels() expectations. Adds WeCom provider suffixes (wecom-bot-id, wecom-secret) and credential env keys to allowlist tests. Updates channels-add-preset to include wecom in supported channel IDs.
Compiler, build-applier, and policy tests
src/lib/messaging/compiler/manifest-compiler.test.ts, test/messaging-build-applier.test.ts, test/messaging-plan-test-helper.ts, test/policies.test.ts, test/policy-tiers.test.ts, test/wecom-policies.test.ts
Extends compiler tests to include wecom in ALL_CHANNELS, adds WeCom credentials to fixtures, validates WeCom credential bindings in plan, network policy entries with wecom_aibot key, build steps for pinned plugin @wecom/wecom-openclaw-plugin@2026.5.25, Hermes render assertions for env/config outputs and absence of WebSocket URLs, and a rejection test for WECOM_DM_POLICY with line breaks. Updates plan helper to decode NEMOCLAW_WECOM_CONFIG_B64 and map dmPolicy/allowed IDs to legacy env vars. Extends build-applier tests for plugin install specs, doctor-env injection, post-doctor config assertions with allowed-IDs encoding, Hermes render coverage for WeCom env/config fields. Updates policy list to include wecom preset, tier tests to assert wecom inclusion in open tier and exclusion in balanced tier. Introduces test/wecom-policies.test.ts regression test loading preset and Hermes YAML, verifying GET /cgi-bin/gettoken is not permitted.

Sequence Diagram(s)

sequenceDiagram
  participant OpenClaw as OpenClaw<br/>(runtime)
  participant Manifest as wecomManifest
  participant Config as channel config<br/>detection
  participant Status as channel-runtime<br/>status
  OpenClaw->>Manifest: lookup wecom channel definition
  Manifest-->>OpenClaw: manifest with configShape: "enabled-flag"
  OpenClaw->>Config: extract enabled channels from openclaw.json
  Config->>Config: check configShape for wecom
  alt configShape === "enabled-flag"
    Config->>Config: look for wecom.enabled === true
  else configShape === "accounts"
    Config->>Config: iterate wecom.accounts[].enabled
  end
  Config-->>Status: [detected channels]
  Status->>Status: probe gateway log for "WeCom" mention
  Status-->>OpenClaw: runtime visibility with configuredChannels, visibleChannels
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Poem

🐰 A new channel hops into the fold,
WeCom's secrets and bot IDs enrolled!
WebSocket proxy now carries the way,
WS_PROXY guides packets each day.
From manifest to policy, all in a row—
The rabbit says: watch those dmPolicy flows! 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% 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
Title check ✅ Passed The PR title 'feat(messaging): add WeCom channel support' clearly and specifically describes the main change: adding WeCom as a new messaging channel with comprehensive support 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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wecom-messaging

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

@github-code-quality

github-code-quality Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the feat/wecom-messaging 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 feat/wecom-messaging 81d9f71 +/-
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 feat/wecom-messaging branch is 47%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main feat/wecom-messaging 81d9f71 +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 71%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 60%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 19%

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

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: messaging-providers-vitest, channels-add-remove-vitest, channels-stop-start-vitest, network-policy-vitest, hermes-sandbox-secret-boundary-vitest, cloud-onboard-vitest
Optional E2E: hermes-e2e-vitest, hermes-root-entrypoint-smoke-vitest, state-backup-restore-vitest, rebuild-hermes-vitest

Dispatch hint: messaging-providers-vitest,channels-add-remove-vitest,channels-stop-start-vitest,network-policy-vitest,hermes-sandbox-secret-boundary-vitest,cloud-onboard-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • messaging-providers-vitest (high): Required because the PR changes messaging credential bindings, provider creation/reuse, placeholder rendering, policy application, and redaction boundaries. This is the highest-signal existing live messaging provider/config E2E.
  • channels-add-remove-vitest (high): Required because a new built-in channel and policy preset can affect channels add/remove, rebuild, registry persistence, provider reuse, and policy-list behavior at the real OpenShell/Docker boundary.
  • channels-stop-start-vitest (high): Required because the PR changes channel availability and OpenClaw/Hermes runtime configuration. The existing matrix validates channel stop/start and rebuild contracts for both supported gateway agents.
  • network-policy-vitest (high): Required because the PR adds WeCom REST/WebSocket egress policy with credential rewrite and modifies tier composition. Network/security boundary changes must be validated with live policy application.
  • hermes-sandbox-secret-boundary-vitest (high): Required because Hermes Docker/start/policy files changed and the WeCom support path relies on credential placeholders, WS/WSS proxy variables, and sandbox secret-boundary assumptions.
  • cloud-onboard-vitest (high): Required because adding a public messaging channel and changing initial policy/channel metadata can affect full hosted onboarding output and the user-facing onboard path.

Optional E2E

  • hermes-e2e-vitest (high): Useful additional confidence for Hermes end-to-end onboard/runtime behavior after Hermes manifest, Dockerfile, start script, proxy env, and messaging support changes.
  • hermes-root-entrypoint-smoke-vitest (medium): Useful focused smoke coverage for Hermes entrypoint/startup after adding WS/WSS proxy exports and supported channel metadata.
  • state-backup-restore-vitest (medium): Useful because OpenClaw manifest durable state directories changed for WeCom. Existing coverage may not be WeCom-specific, but it can catch regressions in backup/restore state handling.
  • rebuild-hermes-vitest (high): Useful adjacent coverage for Hermes rebuild behavior after manifest/policy/start changes and messaging placeholder updates.

New E2E recommendations

  • WeCom messaging channel (high): No existing live E2E appears to exercise WeCom specifically. The current messaging provider/channel jobs cover the shared framework and existing channels, but they do not prove WECOM_BOT_ID/WECOM_SECRET provider creation, OpenClaw/Hermes render output, WebSocket proxy env, WeCom policy hosts, or raw-secret leak prevention for this new channel.
    • Suggested test: Add a WeCom-focused live Vitest E2E that onboards or channels add wecom with fake WECOM_BOT_ID/WECOM_SECRET, verifies OpenShell providers and sandbox-create provider args, checks OpenClaw and Hermes rendered configs contain resolve placeholders only, confirms policy includes openws.work.weixin.qq.com and qyapi.weixin.qq.com with WebSocket/REST credential rewrite, validates WS_PROXY/WSS_PROXY in the sandbox, and exercises rebuild plus stop/start.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: messaging-providers-vitest,channels-add-remove-vitest,channels-stop-start-vitest,network-policy-vitest,hermes-sandbox-secret-boundary-vitest,cloud-onboard-vitest

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: messaging-providers-vitest, channels-add-remove-vitest, network-policy-vitest
Optional Vitest E2E scenarios: channels-stop-start-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=network-policy-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 adds WeCom credential bindings and provider/policy integration through the manifest-first messaging path; this free-standing live Vitest job is the narrowest dispatch for OpenShell provider setup behavior.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=messaging-providers-vitest
  • channels-add-remove-vitest: Channel manifest registration, OpenClaw/Hermes supported-platform metadata, template rendering, and channel add/remove behavior are changed for WeCom, so the live channel add/remove flow should be exercised.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=channels-add-remove-vitest
  • network-policy-vitest: The PR adds a WeCom policy preset, updates policy tiers, and changes Hermes policy additions; this job directly validates live sandbox network-policy application.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=network-policy-vitest

Optional Vitest E2E scenarios

  • channels-stop-start-vitest: Optional adjacent coverage for channel lifecycle commands because runtime channel visibility and channel metadata now recognize WeCom, but the primary changed flow is add/provider/policy setup.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=channels-stop-start-vitest

Relevant changed files

  • agents/hermes/Dockerfile.base
  • agents/hermes/manifest.yaml
  • agents/hermes/policy-additions.yaml
  • agents/hermes/start.sh
  • agents/openclaw/manifest.yaml
  • nemoclaw-blueprint/policies/presets/wecom.yaml
  • nemoclaw-blueprint/policies/tiers.yaml
  • scripts/nemoclaw-start.sh
  • src/lib/channel-runtime-status.ts
  • src/lib/messaging/channels/built-ins.ts
  • src/lib/messaging/channels/metadata.ts
  • src/lib/messaging/channels/template-resolver.ts
  • src/lib/messaging/channels/wecom/manifest.ts
  • src/lib/messaging/channels/wecom/template-resolver.ts
  • src/lib/messaging/manifest/types.ts

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: WeCom WS_PROXY/WSS_PROXY startup exports in OpenClaw and Hermes.
Open items: 0 required · 5 warnings · 1 suggestion · 8 test follow-ups
Since last review: 0 prior items resolved · 5 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: WeCom WS_PROXY/WSS_PROXY startup exports in OpenClaw and Hermes
  • PRA-2 Resolve or justify: Source-of-truth review needed: WeCom policy omission of GET /cgi-bin/gettoken
  • PRA-3 Resolve or justify: Validate WeCom WebSocket proxy and policy enforcement boundaries in scripts/nemoclaw-start.sh:2439
  • PRA-4 Resolve or justify: Update public messaging docs before exposing WeCom in docs/manage-sandboxes/messaging-channels.mdx:105
  • PRA-5 Resolve or justify: Record trust evidence for the new WeCom OpenClaw plugin in src/lib/messaging/channels/wecom/manifest.ts:159
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause
  • PRA-6 In-scope improvement: Move WeCom-specific assertions out of large shared test hotspots

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify security scripts/nemoclaw-start.sh:2439 Add or identify focused runtime/integration validation for the actual boundary, and document the invalid state/source boundary/removal condition adjacent to the WS_PROXY/WSS_PROXY export. At minimum, prove a WeCom-style wss:// handshake routes through OpenShell L7 and prove GET https://qyapi.weixin.qq.com/cgi-bin/gettoken is denied by policy enforcement rather than only absent from YAML.
PRA-4 Resolve/justify docs docs/manage-sandboxes/messaging-channels.mdx:105 Update the messaging guide in this PR, or add an adjacent release-note/docs stub, to describe WeCom's experimental status, required credentials, DM policy and allowlist settings, network preset, onboarding/channel-add examples, and remove or narrow the statement that WeCom is not wired up.
PRA-5 Resolve/justify security src/lib/messaging/channels/wecom/manifest.ts:159 Add reviewable provenance for @wecom/wecom-openclaw-plugin@2026.5.25, or annotate/extend the manifest/build metadata with the trusted source and integrity/advisory review expected for this plugin. If exact npm pins are intentionally the current trust boundary, document that exception adjacent to the manifest. Consider matching the Hermes build-boundary pattern by revalidating OpenClaw package-install outputs against trusted built-in manifests before install.
PRA-6 Improvement architecture Extract cohesive WeCom manifest/template/render assertions into focused channel tests or small helper-driven suites, while leaving shared tests limited to cross-channel contracts.
Review findings by urgency: 0 required fixes, 5 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: WeCom WS_PROXY/WSS_PROXY startup exports in OpenClaw and Hermes

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests assert emitted WS_PROXY/WSS_PROXY strings; they do not prove a websocket handshake uses OpenShell L7.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: scripts/nemoclaw-start.sh exports WS_PROXY/WSS_PROXY around line 2439 and comments on aiohttp around line 2592; agents/hermes/start.sh exports the same variables around line 732.

PRA-2 Resolve/justify — Source-of-truth review needed: WeCom policy omission of GET /cgi-bin/gettoken

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: test/wecom-policies.test.ts proves static YAML absence only.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: nemoclaw-blueprint/policies/presets/wecom.yaml and agents/hermes/policy-additions.yaml include selected qyapi media/message paths but not /cgi-bin/gettoken.

PRA-3 Resolve/justify — Validate WeCom WebSocket proxy and policy enforcement boundaries

  • Location: scripts/nemoclaw-start.sh:2439
  • Category: security
  • Problem: The PR exports WS_PROXY/WSS_PROXY for OpenClaw and Hermes and adds a WeCom policy that intentionally omits GET /cgi-bin/gettoken, but the changed tests only prove environment string emission and static YAML absence. They do not prove that an aiohttp/WeCom-style wss:// client actually uses the OpenShell L7 proxy, that websocket credential rewrite engages, or that the runtime policy engine denies the query-secret gettoken exchange.
  • Impact: A mismatch between client proxy lookup, OpenShell L7 routing, credential rewrite, and policy enforcement could either make WeCom unusable or allow WeCom credential-bearing traffic to bypass the intended sandbox policy boundary.
  • Recommended action: Add or identify focused runtime/integration validation for the actual boundary, and document the invalid state/source boundary/removal condition adjacent to the WS_PROXY/WSS_PROXY export. At minimum, prove a WeCom-style wss:// handshake routes through OpenShell L7 and prove GET https://qyapi.weixin.qq.com/cgi-bin/gettoken is denied by policy enforcement rather than only absent from YAML.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read scripts/nemoclaw-start.sh around lines 2437-2446 and 2590-2608, agents/hermes/start.sh around lines 730-779, test/service-env.test.ts around the WS_PROXY assertions, and test/wecom-policies.test.ts; the shortest current check shows emitted variables and YAML absence, not routed WebSocket or enforced denial.
  • Missing regression test: Add behavior tests named like `WeCom aiohttp websocket handshake uses WSS_PROXY through OpenShell L7` and `WeCom policy engine denies GET https://qyapi.weixin.qq.com/cgi-bin/gettoken\`, observing the proxy/policy boundary rather than only generated env text or YAML.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read scripts/nemoclaw-start.sh around lines 2437-2446 and 2590-2608, agents/hermes/start.sh around lines 730-779, test/service-env.test.ts around the WS_PROXY assertions, and test/wecom-policies.test.ts; the shortest current check shows emitted variables and YAML absence, not routed WebSocket or enforced denial.
  • Evidence: scripts/nemoclaw-start.sh and agents/hermes/start.sh export WS_PROXY/WSS_PROXY and lowercase variants; test/service-env.test.ts asserts those strings. nemoclaw-blueprint/policies/presets/wecom.yaml and agents/hermes/policy-additions.yaml omit GET /cgi-bin/gettoken; test/wecom-policies.test.ts asserts only that omission.

PRA-4 Resolve/justify — Update public messaging docs before exposing WeCom

  • Location: docs/manage-sandboxes/messaging-channels.mdx:105
  • Category: docs
  • Problem: The code registers WeCom for OpenClaw and Hermes and defaults WECOM_DM_POLICY to open, but the public messaging guide still omits WeCom from the frontmatter, overview, requirements table, onboarding list, scripted environment examples, and channel-add examples. It also still says WeCom/Enterprise WeChat is not wired up.
  • Impact: Operators following the guide will not learn that WeCom requires WECOM_BOT_ID and WECOM_SECRET, or that WECOM_ALLOWED_USERS and WECOM_DM_POLICY control direct-message access. Because the code default is open DM access, stale docs can lead to unintentionally broad bot access or mishandled credentials.
  • Recommended action: Update the messaging guide in this PR, or add an adjacent release-note/docs stub, to describe WeCom's experimental status, required credentials, DM policy and allowlist settings, network preset, onboarding/channel-add examples, and remove or narrow the statement that WeCom is not wired up.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read docs/manage-sandboxes/messaging-channels.mdx around lines 6, 16, 64-70, 103-105, 137-197, and 209-212, then compare with src/lib/messaging/channels/wecom/manifest.ts supportedAgents, credentials, and WECOM_DM_POLICY defaultValue.
  • Missing regression test: Existing docs lint/build coverage is sufficient once the prose is updated; no new unit test is needed for this docs-only correction.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read docs/manage-sandboxes/messaging-channels.mdx around lines 6, 16, 64-70, 103-105, 137-197, and 209-212, then compare with src/lib/messaging/channels/wecom/manifest.ts supportedAgents, credentials, and WECOM_DM_POLICY defaultValue.
  • Evidence: src/lib/messaging/channels/wecom/manifest.ts declares supportedAgents ["openclaw", "hermes"], WECOM_BOT_ID/WECOM_SECRET credentials, and WECOM_DM_POLICY defaultValue "open". docs/manage-sandboxes/messaging-channels.mdx still says WeCom/Enterprise WeChat is not wired up.

PRA-5 Resolve/justify — Record trust evidence for the new WeCom OpenClaw plugin

  • Location: src/lib/messaging/channels/wecom/manifest.ts:159
  • Category: security
  • Problem: The WeCom manifest introduces a build-time OpenClaw plugin install, npm:@wecom/wecom-openclaw-plugin@2026.5.25. The exact version pin and argv-style install planning are good, but the changed code does not record package source, checksum/integrity, trusted-publisher status, advisory/license review, or a documented exception for this trust decision. The OpenClaw plugin install collection also appears less hardened than the Hermes package path, which rechecks serialized package specs against trusted built-in manifests at the build boundary.
  • Impact: A compromised, unpublished, or policy-incompatible plugin package would execute in WeCom-enabled sandbox images and receive access to WeCom credential placeholders plus the newly allowed WeCom network policy.
  • Recommended action: Add reviewable provenance for @wecom/wecom-openclaw-plugin@2026.5.25, or annotate/extend the manifest/build metadata with the trusted source and integrity/advisory review expected for this plugin. If exact npm pins are intentionally the current trust boundary, document that exception adjacent to the manifest. Consider matching the Hermes build-boundary pattern by revalidating OpenClaw package-install outputs against trusted built-in manifests before install.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/messaging/channels/wecom/manifest.ts around lines 154-161 and src/lib/messaging/applier/build/messaging-build-applier.mts around collectOpenClawMessagingPluginInstalls and trustedHermesUvPackageSpecsForPlan; compare the WeCom package pin with the absence of adjacent provenance or an OpenClaw trusted-spec recheck.
  • Missing regression test: Add a build-applier test named `OpenClaw messaging plugin installs reject package specs not declared by active trusted manifests` if the build-boundary recheck is added; otherwise document the provenance exception and rely on existing package-install planning tests.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/messaging/channels/wecom/manifest.ts around lines 154-161 and src/lib/messaging/applier/build/messaging-build-applier.mts around collectOpenClawMessagingPluginInstalls and trustedHermesUvPackageSpecsForPlan; compare the WeCom package pin with the absence of adjacent provenance or an OpenClaw trusted-spec recheck.
  • Evidence: The manifest installs `npm:@wecom/wecom-openclaw-plugin@${WECOM_OPENCLAW_PLUGIN_VERSION}` with version 2026.5.25. collectOpenClawMessagingPluginInstalls reads serialized package-install outputs and resolves specs, while the nearby Hermes path derives a trusted spec set from built-in manifests.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-6 Improvement — Move WeCom-specific assertions out of large shared test hotspots

  • Location: not file-specific
  • Category: architecture
  • Problem: The PR adds substantial WeCom-specific assertions to already-large shared suites. The channel has its own manifest and resolver directory, so many channel-shape assertions can live in focused WeCom tests while shared tests keep only registry ordering, metadata derivation, applier contracts, and runtime visibility contracts.
  • Impact: Continuing to grow large shared suites makes future channel additions harder to review and increases merge-conflict risk with the many active messaging-channel PRs touching the same files.
  • Suggested action: Extract cohesive WeCom manifest/template/render assertions into focused channel tests or small helper-driven suites, while leaving shared tests limited to cross-channel contracts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect src/lib/messaging/channels/manifests.test.ts, src/lib/messaging/compiler/manifest-compiler.test.ts, src/lib/messaging/applier/setup-applier.test.ts, and src/lib/channel-runtime-status.test.ts; the synthetic size budget reports +97, +92, +66, and +44 line growth in these hotspots.
  • Missing regression test: No new behavior coverage is required; preserve the existing WeCom assertions while relocating them so the same manifest, render, compiler, and applier expectations still execute.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Monolith growth was reported for src/lib/messaging/channels/manifests.test.ts at 910 lines, manifest-compiler.test.ts at 1498 lines, setup-applier.test.ts at 907 lines, and channel-runtime-status.test.ts at 497 lines.
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-6 shrink: WeCom-specific manifest/render/compiler assertions embedded in large shared suites.
    • Replacement: Focused WeCom channel tests plus minimal shared cross-channel contract assertions.
    • Safety boundary: Do not remove security-sensitive coverage for credential placeholders, policy keys, package install planning, runtime visibility, or path/prototype-pollution validation.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — WeCom aiohttp websocket handshake uses WSS_PROXY through OpenShell L7. The PR changes sandbox startup proxy variables, network policy presets, credential rewrite assumptions, agent manifests, and build-time plugin installation. Static tests cover manifest/render/planning shape, but runtime validation is recommended for the actual proxy and policy enforcement boundaries.
  • PRA-T2 Runtime validation — WeCom policy engine denies GET https://qyapi.weixin.qq.com/cgi-bin/gettoken. The PR changes sandbox startup proxy variables, network policy presets, credential rewrite assumptions, agent manifests, and build-time plugin installation. Static tests cover manifest/render/planning shape, but runtime validation is recommended for the actual proxy and policy enforcement boundaries.
  • PRA-T3 Runtime validation — WeCom policy engine allows only declared qyapi media/message paths and rejects undeclared query-secret token exchange. The PR changes sandbox startup proxy variables, network policy presets, credential rewrite assumptions, agent manifests, and build-time plugin installation. Static tests cover manifest/render/planning shape, but runtime validation is recommended for the actual proxy and policy enforcement boundaries.
  • PRA-T4 Runtime validation — OpenClaw messaging plugin installs reject package specs not declared by active trusted manifests. The PR changes sandbox startup proxy variables, network policy presets, credential rewrite assumptions, agent manifests, and build-time plugin installation. Static tests cover manifest/render/planning shape, but runtime validation is recommended for the actual proxy and policy enforcement boundaries.
  • PRA-T5 Runtime validation — WeCom render omits allowFrom when WECOM_ALLOWED_USERS is empty and preserves allowFrom array when populated. The PR changes sandbox startup proxy variables, network policy presets, credential rewrite assumptions, agent manifests, and build-time plugin installation. Static tests cover manifest/render/planning shape, but runtime validation is recommended for the actual proxy and policy enforcement boundaries.
  • PRA-T6 Acceptance clause — Refs feat(messaging): onboard additional experimental messaging channels #5492 — add test evidence or identify existing coverage. The deterministic context did not include linked issue body or comments, so no literal issue acceptance clauses were available to verify.
  • PRA-T7 Acceptance clause — Add WeCom OpenShell policy presets for OpenClaw and Hermes, including the WeCom AI Bot websocket and REST endpoints. — add test evidence or identify existing coverage. nemoclaw-blueprint/policies/presets/wecom.yaml and agents/hermes/policy-additions.yaml add websocket and REST endpoints. Runtime policy-engine enforcement for the intentionally denied gettoken path is not proven by changed tests.
  • PRA-T8 Acceptance clause — Export `WS_PROXY`/`WSS_PROXY` alongside the existing HTTP proxy variables for runtime websocket clients. — add test evidence or identify existing coverage. scripts/nemoclaw-start.sh and agents/hermes/start.sh export WS_PROXY/WSS_PROXY and lowercase variants; test/service-env.test.ts checks emitted variables. No changed test proves a websocket client uses those variables through OpenShell L7.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: WeCom WS_PROXY/WSS_PROXY startup exports in OpenClaw and Hermes

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests assert emitted WS_PROXY/WSS_PROXY strings; they do not prove a websocket handshake uses OpenShell L7.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: scripts/nemoclaw-start.sh exports WS_PROXY/WSS_PROXY around line 2439 and comments on aiohttp around line 2592; agents/hermes/start.sh exports the same variables around line 732.

PRA-2 Resolve/justify — Source-of-truth review needed: WeCom policy omission of GET /cgi-bin/gettoken

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: test/wecom-policies.test.ts proves static YAML absence only.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: nemoclaw-blueprint/policies/presets/wecom.yaml and agents/hermes/policy-additions.yaml include selected qyapi media/message paths but not /cgi-bin/gettoken.

PRA-3 Resolve/justify — Validate WeCom WebSocket proxy and policy enforcement boundaries

  • Location: scripts/nemoclaw-start.sh:2439
  • Category: security
  • Problem: The PR exports WS_PROXY/WSS_PROXY for OpenClaw and Hermes and adds a WeCom policy that intentionally omits GET /cgi-bin/gettoken, but the changed tests only prove environment string emission and static YAML absence. They do not prove that an aiohttp/WeCom-style wss:// client actually uses the OpenShell L7 proxy, that websocket credential rewrite engages, or that the runtime policy engine denies the query-secret gettoken exchange.
  • Impact: A mismatch between client proxy lookup, OpenShell L7 routing, credential rewrite, and policy enforcement could either make WeCom unusable or allow WeCom credential-bearing traffic to bypass the intended sandbox policy boundary.
  • Recommended action: Add or identify focused runtime/integration validation for the actual boundary, and document the invalid state/source boundary/removal condition adjacent to the WS_PROXY/WSS_PROXY export. At minimum, prove a WeCom-style wss:// handshake routes through OpenShell L7 and prove GET https://qyapi.weixin.qq.com/cgi-bin/gettoken is denied by policy enforcement rather than only absent from YAML.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read scripts/nemoclaw-start.sh around lines 2437-2446 and 2590-2608, agents/hermes/start.sh around lines 730-779, test/service-env.test.ts around the WS_PROXY assertions, and test/wecom-policies.test.ts; the shortest current check shows emitted variables and YAML absence, not routed WebSocket or enforced denial.
  • Missing regression test: Add behavior tests named like `WeCom aiohttp websocket handshake uses WSS_PROXY through OpenShell L7` and `WeCom policy engine denies GET https://qyapi.weixin.qq.com/cgi-bin/gettoken\`, observing the proxy/policy boundary rather than only generated env text or YAML.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read scripts/nemoclaw-start.sh around lines 2437-2446 and 2590-2608, agents/hermes/start.sh around lines 730-779, test/service-env.test.ts around the WS_PROXY assertions, and test/wecom-policies.test.ts; the shortest current check shows emitted variables and YAML absence, not routed WebSocket or enforced denial.
  • Evidence: scripts/nemoclaw-start.sh and agents/hermes/start.sh export WS_PROXY/WSS_PROXY and lowercase variants; test/service-env.test.ts asserts those strings. nemoclaw-blueprint/policies/presets/wecom.yaml and agents/hermes/policy-additions.yaml omit GET /cgi-bin/gettoken; test/wecom-policies.test.ts asserts only that omission.

PRA-4 Resolve/justify — Update public messaging docs before exposing WeCom

  • Location: docs/manage-sandboxes/messaging-channels.mdx:105
  • Category: docs
  • Problem: The code registers WeCom for OpenClaw and Hermes and defaults WECOM_DM_POLICY to open, but the public messaging guide still omits WeCom from the frontmatter, overview, requirements table, onboarding list, scripted environment examples, and channel-add examples. It also still says WeCom/Enterprise WeChat is not wired up.
  • Impact: Operators following the guide will not learn that WeCom requires WECOM_BOT_ID and WECOM_SECRET, or that WECOM_ALLOWED_USERS and WECOM_DM_POLICY control direct-message access. Because the code default is open DM access, stale docs can lead to unintentionally broad bot access or mishandled credentials.
  • Recommended action: Update the messaging guide in this PR, or add an adjacent release-note/docs stub, to describe WeCom's experimental status, required credentials, DM policy and allowlist settings, network preset, onboarding/channel-add examples, and remove or narrow the statement that WeCom is not wired up.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read docs/manage-sandboxes/messaging-channels.mdx around lines 6, 16, 64-70, 103-105, 137-197, and 209-212, then compare with src/lib/messaging/channels/wecom/manifest.ts supportedAgents, credentials, and WECOM_DM_POLICY defaultValue.
  • Missing regression test: Existing docs lint/build coverage is sufficient once the prose is updated; no new unit test is needed for this docs-only correction.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read docs/manage-sandboxes/messaging-channels.mdx around lines 6, 16, 64-70, 103-105, 137-197, and 209-212, then compare with src/lib/messaging/channels/wecom/manifest.ts supportedAgents, credentials, and WECOM_DM_POLICY defaultValue.
  • Evidence: src/lib/messaging/channels/wecom/manifest.ts declares supportedAgents ["openclaw", "hermes"], WECOM_BOT_ID/WECOM_SECRET credentials, and WECOM_DM_POLICY defaultValue "open". docs/manage-sandboxes/messaging-channels.mdx still says WeCom/Enterprise WeChat is not wired up.

PRA-5 Resolve/justify — Record trust evidence for the new WeCom OpenClaw plugin

  • Location: src/lib/messaging/channels/wecom/manifest.ts:159
  • Category: security
  • Problem: The WeCom manifest introduces a build-time OpenClaw plugin install, npm:@wecom/wecom-openclaw-plugin@2026.5.25. The exact version pin and argv-style install planning are good, but the changed code does not record package source, checksum/integrity, trusted-publisher status, advisory/license review, or a documented exception for this trust decision. The OpenClaw plugin install collection also appears less hardened than the Hermes package path, which rechecks serialized package specs against trusted built-in manifests at the build boundary.
  • Impact: A compromised, unpublished, or policy-incompatible plugin package would execute in WeCom-enabled sandbox images and receive access to WeCom credential placeholders plus the newly allowed WeCom network policy.
  • Recommended action: Add reviewable provenance for @wecom/wecom-openclaw-plugin@2026.5.25, or annotate/extend the manifest/build metadata with the trusted source and integrity/advisory review expected for this plugin. If exact npm pins are intentionally the current trust boundary, document that exception adjacent to the manifest. Consider matching the Hermes build-boundary pattern by revalidating OpenClaw package-install outputs against trusted built-in manifests before install.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/messaging/channels/wecom/manifest.ts around lines 154-161 and src/lib/messaging/applier/build/messaging-build-applier.mts around collectOpenClawMessagingPluginInstalls and trustedHermesUvPackageSpecsForPlan; compare the WeCom package pin with the absence of adjacent provenance or an OpenClaw trusted-spec recheck.
  • Missing regression test: Add a build-applier test named `OpenClaw messaging plugin installs reject package specs not declared by active trusted manifests` if the build-boundary recheck is added; otherwise document the provenance exception and rely on existing package-install planning tests.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/messaging/channels/wecom/manifest.ts around lines 154-161 and src/lib/messaging/applier/build/messaging-build-applier.mts around collectOpenClawMessagingPluginInstalls and trustedHermesUvPackageSpecsForPlan; compare the WeCom package pin with the absence of adjacent provenance or an OpenClaw trusted-spec recheck.
  • Evidence: The manifest installs `npm:@wecom/wecom-openclaw-plugin@${WECOM_OPENCLAW_PLUGIN_VERSION}` with version 2026.5.25. collectOpenClawMessagingPluginInstalls reads serialized package-install outputs and resolves specs, while the nearby Hermes path derives a trusted spec set from built-in manifests.

PRA-6 Improvement — Move WeCom-specific assertions out of large shared test hotspots

  • Location: not file-specific
  • Category: architecture
  • Problem: The PR adds substantial WeCom-specific assertions to already-large shared suites. The channel has its own manifest and resolver directory, so many channel-shape assertions can live in focused WeCom tests while shared tests keep only registry ordering, metadata derivation, applier contracts, and runtime visibility contracts.
  • Impact: Continuing to grow large shared suites makes future channel additions harder to review and increases merge-conflict risk with the many active messaging-channel PRs touching the same files.
  • Suggested action: Extract cohesive WeCom manifest/template/render assertions into focused channel tests or small helper-driven suites, while leaving shared tests limited to cross-channel contracts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect src/lib/messaging/channels/manifests.test.ts, src/lib/messaging/compiler/manifest-compiler.test.ts, src/lib/messaging/applier/setup-applier.test.ts, and src/lib/channel-runtime-status.test.ts; the synthetic size budget reports +97, +92, +66, and +44 line growth in these hotspots.
  • Missing regression test: No new behavior coverage is required; preserve the existing WeCom assertions while relocating them so the same manifest, render, compiler, and applier expectations still execute.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Monolith growth was reported for src/lib/messaging/channels/manifests.test.ts at 910 lines, manifest-compiler.test.ts at 1498 lines, setup-applier.test.ts at 907 lines, and channel-runtime-status.test.ts at 497 lines.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
scripts/nemoclaw-start.sh (1)

2439-2445: 🩺 Stability & Availability | 🔵 Trivial

Run sandbox entrypoint E2E coverage for this proxy-env update.

Given this touches the sandbox start script, run the recommended lanes (sandbox-survival-e2e, sandbox-operations-e2e, cloud-e2e, openclaw-slack-pairing-e2e) to validate boot/recovery behavior end to end.
As per path instructions, scripts/nemoclaw-start.sh changes should be validated via the listed sandbox E2E workflows.

Also applies to: 2592-2593, 2604-2610

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/nemoclaw-start.sh` around lines 2439 - 2445, The changes to the proxy
environment variables (WS_PROXY, WSS_PROXY, NO_PROXY, http_proxy, https_proxy,
ws_proxy, wss_proxy exports) in the nemoclaw-start.sh script need validation
through end-to-end testing. Run the recommended E2E test lanes
(sandbox-survival-e2e, sandbox-operations-e2e, cloud-e2e, and
openclaw-slack-pairing-e2e) to validate that the sandbox boots and recovers
correctly with these proxy environment variable changes. This ensures the proxy
configuration does not break sandbox initialization or operational behavior.

Source: Path instructions

agents/hermes/start.sh (1)

726-732: 🩺 Stability & Availability | 🔵 Trivial

Run the Hermes E2E lanes for this entrypoint proxy-env change.

This is a runtime-entrypoint behavior change, so validating with the Hermes E2E set is worthwhile (hermes-e2e, hermes-inference-switch-e2e, hermes-discord-e2e, hermes-slack-e2e, hermes-onboard-security-posture-e2e, rebuild-hermes-e2e, rebuild-hermes-stale-base-e2e).
As per path instructions, changes under agents/hermes/** should be validated with the listed Hermes E2E jobs.

Also applies to: 765-771

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agents/hermes/start.sh` around lines 726 - 732, The proxy environment
variable changes in the start.sh script (lines 726-732 and 765-771) are
runtime-entrypoint behavior changes and need to be validated. Run the following
Hermes E2E test suites to validate this change: hermes-e2e,
hermes-inference-switch-e2e, hermes-discord-e2e, hermes-slack-e2e,
hermes-onboard-security-posture-e2e, rebuild-hermes-e2e, and
rebuild-hermes-stale-base-e2e. This will ensure the proxy configuration works
correctly across different Hermes scenarios and integrations.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@agents/hermes/start.sh`:
- Around line 726-732: The proxy environment variable changes in the start.sh
script (lines 726-732 and 765-771) are runtime-entrypoint behavior changes and
need to be validated. Run the following Hermes E2E test suites to validate this
change: hermes-e2e, hermes-inference-switch-e2e, hermes-discord-e2e,
hermes-slack-e2e, hermes-onboard-security-posture-e2e, rebuild-hermes-e2e, and
rebuild-hermes-stale-base-e2e. This will ensure the proxy configuration works
correctly across different Hermes scenarios and integrations.

In `@scripts/nemoclaw-start.sh`:
- Around line 2439-2445: The changes to the proxy environment variables
(WS_PROXY, WSS_PROXY, NO_PROXY, http_proxy, https_proxy, ws_proxy, wss_proxy
exports) in the nemoclaw-start.sh script need validation through end-to-end
testing. Run the recommended E2E test lanes (sandbox-survival-e2e,
sandbox-operations-e2e, cloud-e2e, and openclaw-slack-pairing-e2e) to validate
that the sandbox boots and recovers correctly with these proxy environment
variable changes. This ensures the proxy configuration does not break sandbox
initialization or operational behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8be642c9-f344-418b-af80-da6fc9b5025c

📥 Commits

Reviewing files that changed from the base of the PR and between a9f31e4 and b660048.

📒 Files selected for processing (36)
  • agents/hermes/Dockerfile.base
  • agents/hermes/manifest.yaml
  • agents/hermes/policy-additions.yaml
  • agents/hermes/start.sh
  • agents/openclaw/manifest.yaml
  • nemoclaw-blueprint/policies/presets/wecom.yaml
  • nemoclaw-blueprint/policies/tiers.yaml
  • scripts/nemoclaw-start.sh
  • src/lib/agent/defs.test.ts
  • src/lib/channel-runtime-status.test.ts
  • src/lib/messaging-channel-config.test.ts
  • src/lib/messaging/AGENTS.md
  • src/lib/messaging/applier/setup-applier.test.ts
  • src/lib/messaging/channels/built-ins.ts
  • src/lib/messaging/channels/manifests.test.ts
  • src/lib/messaging/channels/metadata.test.ts
  • src/lib/messaging/channels/template-resolver.ts
  • src/lib/messaging/channels/wecom/manifest.ts
  • src/lib/messaging/channels/wecom/template-resolver.ts
  • src/lib/messaging/compiler/manifest-compiler.test.ts
  • src/lib/messaging/diagnostics.test.ts
  • src/lib/onboard/extra-placeholder-keys.test.ts
  • src/lib/onboard/initial-policy.test.ts
  • src/lib/onboard/messaging-channel-setup.test.ts
  • src/lib/onboard/messaging-prep.test.ts
  • src/lib/onboard/messaging-reuse.test.ts
  • src/lib/sandbox/channels.test.ts
  • test/channels-add-preset.test.ts
  • test/credentials.test.ts
  • test/hermes-start.test.ts
  • test/messaging-build-applier.test.ts
  • test/messaging-plan-test-helper.ts
  • test/policies.test.ts
  • test/policy-tiers.test.ts
  • test/sandbox-provider-cleanup.test.ts
  • test/service-env.test.ts

@sandl99 sandl99 added area: messaging Messaging channels, bridges, manifests, or channel lifecycle enhancement New capability or improvement request VRDC Issues and PRs submitted by NVIDIA VRDC test team. labels Jun 23, 2026
Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99

sandl99 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

PR Review Advisor warning justifications:

  • PRA-1 / PRA-4: Keep as-is. WS_PROXY/WSS_PROXY are exported from the same OpenShell proxy source as HTTP_PROXY/HTTPS_PROXY so WeCom WebSocket clients can use the enforced policy path. Existing startup tests cover env emission and NO_PROXY includes loopback/gateway bypass entries; deeper route behavior belongs in runtime/E2E validation, not this manifest wiring PR.
  • PRA-2 / PRA-3: Keep as-is. WeCom follows the current OpenClaw messaging default: DM access is open unless the operator configures an allowlist/policy. The PR already exposes WECOM_DM_POLICY and WECOM_ALLOWED_USERS so operators can restrict access without changing the default behavior.
  • PRA-5: Justified. GET /cgi-bin/gettoken was removed from both policy surfaces, so the query-secret Agent token exchange is not allowed by this PR. The remaining REST/WebSocket policy covers the AI Bot paths used by the rendered Bot ID/Secret configuration.
  • PRA-6: Justified. The OpenClaw plugin package is exact-version pinned as npm:@wecom/wecom-openclaw-plugin@2026.5.25; package provenance/advisory metadata is not currently part of the channel manifest schema. Adding a repo-wide package trust metadata contract should be handled separately.
  • PRA-7: Deferred intentionally. Docs are not being updated in this PR yet; WeCom docs will be handled in the later docs-update window so this feature PR stays code/test/policy scoped.
  • PRA-8 / PRA-T*: Deferred. Existing tests cover manifest registration, render output, runtime visibility, policy presets, and package install planning. Further E2E/runtime route validation and test-file extraction can be handled as follow-up hardening.

@sandl99
sandl99 requested review from cv and ericksoa June 23, 2026 16:53
@NVIDIA NVIDIA deleted a comment from github-actions Bot Jun 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28040899261
Target ref: feat/wecom-messaging
Requested jobs: messaging-providers-e2e,channels-add-remove-e2e,channels-stop-start-hermes-e2e,network-policy-e2e,hermes-root-entrypoint-smoke-e2e,rebuild-hermes-e2e,channels-stop-start-openclaw-e2e,rebuild-openclaw-e2e,sandbox-survival-e2e,tunnel-lifecycle-e2e
Summary: 10 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
hermes-root-entrypoint-smoke-e2e ✅ success
messaging-providers-e2e ✅ success
network-policy-e2e ✅ success
rebuild-hermes-e2e ✅ success
rebuild-openclaw-e2e ✅ success
sandbox-survival-e2e ✅ success
tunnel-lifecycle-e2e ✅ success

@jyaunches jyaunches added v0.0.68 and removed v0.0.67 labels Jun 24, 2026
@jyaunches jyaunches removed the v0.0.68 label Jun 25, 2026
@sandl99
sandl99 marked this pull request as draft June 26, 2026 05:58
@copy-pr-bot

copy-pr-bot Bot commented Jun 26, 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.

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 enhancement New capability or improvement request VRDC Issues and PRs submitted by NVIDIA VRDC test team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants