Skip to content

feat(messaging): add Zalo Bot API channel#5583

Open
hunglp6d wants to merge 12 commits into
mainfrom
feat/messaging-channel-zalo
Open

feat(messaging): add Zalo Bot API channel#5583
hunglp6d wants to merge 12 commits into
mainfrom
feat/messaging-channel-zalo

Conversation

@hunglp6d

@hunglp6d hunglp6d commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Zalo (Bot API) as a messaging channel for sandboxed OpenClaw agents, built on the manifest-first messaging architecture (#3896). The channel renders the flat single-account channels.zalo config the upstream @openclaw/zalo plugin validates, ships a scoped zalo network policy preset, installs the plugin at image build, and runs an OpenClaw bridge-health check. Verified end-to-end on a live sandbox — the bot pairs, receives, and replies.

Related Issue

Part of #5492

Result

image

Changes

  • New channel manifest src/lib/messaging/channels/zalo/manifest.ts — OpenClaw-only, token-paste auth, ZALO_BOT_TOKEN credential → {sandbox}-zalo-bridge provider, ZALO_ALLOWED_IDS DM allowlist, ZALO_GROUP_POLICY.
  • Renders the flat channels.zalo shape (botToken/proxy/dmPolicy/allowFrom/groupPolicy) the @openclaw/zalo plugin expects — not the Telegram-style accounts.default nesting (which the plugin schema rejects).
  • Template resolver channels/zalo/template-resolver.ts (proxy URL, allowlist, derived dmPolicy) and an OpenClaw bridge-health hook under channels/zalo/hooks/.
  • New nemoclaw-blueprint/policies/presets/zalo.yaml allowing bot-api.zaloplatforms.com:443 (long-polling; no inbound webhook).
  • Registered the manifest/resolver/hooks in built-ins.ts, channels/template-resolver.ts, hooks/builtins.ts; added zalo to agents/openclaw/manifest.yaml platforms.
  • Widened selectManifests typing in metadata.ts to support the first OpenClaw-only channel.
  • Updated channel/preset/provider/platform enumeration tests for the new channel.
  • Docs: messaging channels, integration policy examples, commands reference (+ generated Hermes variant).

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: Hung Le hple@nvidia.com

Summary by CodeRabbit

  • New Features

    • Zalo is now available as a supported messaging platform for agent configuration and user communications.
    • Users can configure Zalo messaging channels with secure bot token authentication, allowlist-based user ID management, and flexible group messaging policies.
  • Tests

    • Comprehensive test suite added covering Zalo channel setup, enrollment workflows, health monitoring, and runtime behavior validation.

@copy-pr-bot

copy-pr-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

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

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a11d0f1e-8973-454e-a7f1-0bfd854cfe23

📥 Commits

Reviewing files that changed from the base of the PR and between e88ccaa and e129caf.

📒 Files selected for processing (1)
  • src/lib/messaging/compiler/manifest-compiler.test.ts

📝 Walkthrough

Walkthrough

Adds Zalo as a sixth built-in messaging channel for the OpenClaw agent. The change introduces a full channel manifest, a network policy preset for bot-api.zaloplatforms.com, a template resolver, bridge health hook wiring, and registers Zalo in all relevant built-in registries. Test suites are updated throughout to cover the new channel.

Changes

Zalo Channel Integration

Layer / File(s) Summary
Zalo manifest and network policy preset
src/lib/messaging/channels/zalo/manifest.ts, nemoclaw-blueprint/policies/presets/zalo.yaml, agents/openclaw/manifest.yaml, src/lib/messaging/channels/built-ins.ts, src/lib/messaging/channels/metadata.ts
Defines the full zaloManifest with token-paste auth, enrollment inputs (bot token, allowed IDs, group policy), OpenClaw JSON render fragments, state persistence, and hook registrations. Adds the network policy preset allowing GET/POST to bot-api.zaloplatforms.com:443. Registers zaloManifest in BUILT_IN_CHANNEL_MANIFESTS and adds zalo to the agent's supported platforms list.
Zalo template resolver and hook registrations
src/lib/messaging/channels/zalo/template-resolver.ts, src/lib/messaging/channels/template-resolver.ts, src/lib/messaging/channels/zalo/hooks/openclaw-bridge-health.ts, src/lib/messaging/channels/zalo/hooks/index.ts, src/lib/messaging/hooks/builtins.ts
Adds resolveZaloTemplateReference for proxy URL, allowed user IDs, DM policy, and group policy resolution, registering it in BUILT_IN_TEMPLATE_REFERENCE_RESOLVERS. Adds createZaloOpenClawBridgeHealthHookRegistration and createZaloHookRegistrations, wiring them into BuiltInMessagingHookOptions and createBuiltInMessagingHookRegistrations.
Unit and integration test updates
src/lib/messaging/channels/manifests.test.ts, src/lib/messaging/channels/metadata.test.ts, src/lib/messaging/compiler/manifest-compiler.test.ts, test/channels-add-preset.test.ts, src/lib/messaging/hooks/hook-runner.test.ts, src/lib/sandbox/channels.test.ts, test/policies.test.ts, test/sandbox-provider-cleanup.test.ts, src/lib/messaging/diagnostics.test.ts, src/lib/onboard/messaging-prep.test.ts, src/lib/messaging-channel-config.test.ts, src/lib/agent/defs.test.ts
Extends assertions across all affected test suites to include zalo: channel IDs, env keys (ZALO_BOT_TOKEN, ZALO_ALLOWED_IDS, ZALO_GROUP_POLICY), hook ID (zalo.openclawBridgeHealth), render shape, credential bindings, build steps, health checks, policy preset list, and provider suffix zalo-bridge.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • NVIDIA/NemoClaw#5135: Both PRs modify prepareCreateSandboxMessaging test expectations; this PR adds ZALO_BOT_TOKEN to the same assertion updated in that refactor.
  • NVIDIA/NemoClaw#5338: This PR extends the same BUILT_IN_CHANNEL_MANIFESTS array and built-in hook registration plumbing introduced by that manifest-driven messaging refactor.
  • NVIDIA/NemoClaw#5463: This PR's metadata test updates (managed channel names, env key derivation) depend on the channel metadata helpers added in that PR.

Suggested labels

integration: openclaw

Suggested reviewers

  • cv

🐇 A new friend hops onto the wire,
Zalo joins the channel choir!
Bot token pasted, policy set,
allowlist guards each DM yet.
From Telegram to Zalo's light —
six channels strong, the stack feels right! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% 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 'feat(messaging): add Zalo Bot API channel' accurately and concisely describes the main change: adding Zalo as a new messaging channel. It clearly summarizes the primary objective of the changeset.
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 feat/messaging-channel-zalo

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

@github-code-quality

github-code-quality Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File a2b39a4 +/-
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 branch is 46%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File a2b39a4 +/-
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 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

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

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jun 22, 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: Zalo flat `channels.zalo` OpenClaw render.
Open items: 0 required · 9 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 8 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: Zalo flat `channels.zalo` OpenClaw render
  • PRA-2 Resolve or justify: Source-of-truth review needed: Zalo group-policy render default
  • PRA-3 Resolve or justify: Source-of-truth review needed: Zalo Bot API policy wildcard
  • PRA-4 Resolve or justify: Source-of-truth review needed: Zalo OpenClaw plugin install
  • PRA-5 Resolve or justify: Zalo bridge trust-boundary validation and egress policy are too broad in src/lib/messaging/channels/zalo/manifest.ts:24
  • PRA-6 Resolve or justify: Zalo authorization render coverage stops before the allowlist path in src/lib/messaging/compiler/manifest-compiler.test.ts:339
  • PRA-7 Resolve or justify: `channels add zalo` lacks workflow-side provider binding evidence in test/channels-add-preset.test.ts:316
  • PRA-8 Resolve or justify: Flat `channels.zalo` compatibility needs a source boundary and removal condition in src/lib/messaging/channels/zalo/manifest.ts:83
  • PRA-9 Resolve or justify: New `@openclaw/zalo` installer boundary needs a local trust contract in src/lib/messaging/channels/zalo/manifest.ts:119
  • 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: Zalo authorization render coverage stops before the allowlist path
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

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 architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-4 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-5 Resolve/justify security src/lib/messaging/channels/zalo/manifest.ts:24 Add manifest `formatPattern`/`formatHint` for `ZALO_BOT_TOKEN` and comma-separated numeric `ZALO_ALLOWED_IDS`, mirror Telegram's render-time group-policy allowlist with a safe `allowlist` fallback or explicit refusal, and narrow `zalo.yaml` to the required Bot API routes or add an in-repo policy comment/test proving why `/**` is required.
PRA-6 Resolve/justify tests src/lib/messaging/compiler/manifest-compiler.test.ts:339 Add a focused Zalo compiler case with numeric `ZALO_ALLOWED_IDS` and assert the final `channels.zalo` value contains `dmPolicy: "allowlist"` and the expected deduplicated numeric `allowFrom` array while remaining flat with no `accounts` key.
PRA-7 Resolve/justify workflow test/channels-add-preset.test.ts:316 Extend the Zalo case in the existing add-channel harness, or add a focused Zalo test, to assert `providerCalls` contains `test-sb-zalo-bridge` with env key `ZALO_BOT_TOKEN` before the rebuild prompt.
PRA-8 Resolve/justify architecture src/lib/messaging/channels/zalo/manifest.ts:83 Expand the manifest comment or nearby test comment to identify the upstream `@openclaw/zalo` schema as the source boundary and state the removal condition, for example that the flat render can be revisited once the plugin accepts the shared `accounts.default` schema.
PRA-9 Resolve/justify security src/lib/messaging/channels/zalo/manifest.ts:119 Add a short in-code or test comment next to `agentPackages` identifying the trusted upstream package/source contract and why the `{{openclaw.version}}` pin is the intended compatibility boundary for Zalo.
Review findings by urgency: 0 required fixes, 9 items to resolve/justify, 0 in-scope improvements

⚠️ 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: Zalo flat `channels.zalo` OpenClaw render

  • 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: Existing manifest/compiler tests assert `path: "channels.zalo"` and no `accounts` key.
  • 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: `src/lib/messaging/channels/zalo/manifest.ts` comments on the rejection and `src/lib/messaging/compiler/manifest-compiler.test.ts` pins the flat render, but neither records a removal condition.

PRA-2 Resolve/justify — Source-of-truth review needed: Zalo group-policy render default

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • 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: No current Zalo test covers invalid persisted/direct `ZALO_GROUP_POLICY` render behavior.
  • 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: `src/lib/messaging/channels/zalo/template-resolver.ts` returns `nonEmptyString(stateValue(context, "zaloConfig.groupPolicy")) ?? "allowlist"`.

PRA-3 Resolve/justify — Source-of-truth review needed: Zalo Bot API policy wildcard

  • 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: No current policy test pins a narrowed route set or records a wildcard justification for Zalo.
  • 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/zalo.yaml` says long-polling `getUpdates` is GET and sending messages is POST, but both rules use `path: "/**"`.

PRA-4 Resolve/justify — Source-of-truth review needed: Zalo OpenClaw plugin install

  • 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: Existing tests assert the package spec; a comment-level contract is the missing evidence unless maintainers want a dedicated documentation assertion.
  • 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: `zaloManifest.agentPackages` sets `spec: "npm:@openclaw/zalo@{{openclaw.version}}"`, `pin: true`, and `required: true` without a nearby source/version contract.

PRA-5 Resolve/justify — Zalo bridge trust-boundary validation and egress policy are too broad

  • Location: src/lib/messaging/channels/zalo/manifest.ts:24
  • Category: security
  • Problem: The Zalo manifest documents `ZALO_BOT_TOKEN` as `id:secret` and `ZALO_ALLOWED_IDS` as numeric IDs, but neither input declares a `formatPattern`/`formatHint`, so the shared token/config hooks cannot enforce those contracts. The resolver also renders any non-empty `zaloConfig.groupPolicy` instead of rechecking `open|allowlist|disabled`, while the new policy preset allows Node GET and POST to every path on `bot-api.zaloplatforms.com`.
  • Impact: Malformed credentials or non-numeric authorization entries can be saved and rendered into OpenClaw, corrupt direct/persisted group-policy state can reach the plugin, and the sandbox receives broader Bot API egress than the documented long-polling/send-message flow may require.
  • Recommended action: Add manifest `formatPattern`/`formatHint` for `ZALO_BOT_TOKEN` and comma-separated numeric `ZALO_ALLOWED_IDS`, mirror Telegram's render-time group-policy allowlist with a safe `allowlist` fallback or explicit refusal, and narrow `zalo.yaml` to the required Bot API routes or add an in-repo policy comment/test proving why `/**` is required.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/messaging/channels/zalo/manifest.ts` inputs for absent `formatPattern`, `src/lib/messaging/channels/zalo/template-resolver.ts` `zaloGroupPolicy()`, and `nemoclaw-blueprint/policies/presets/zalo.yaml` rules for GET/POST `"/**"`.
  • Missing regression test: Add tests named `rejects malformed ZALO_BOT_TOKEN before saving the Zalo provider`, `rejects non-numeric ZALO_ALLOWED_IDS before rendering Zalo allowFrom`, `falls back to allowlist for invalid persisted ZALO_GROUP_POLICY`, and `zalo policy preset limits runtime access to documented Bot API routes or records the wildcard justification`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/messaging/channels/zalo/manifest.ts` inputs for absent `formatPattern`, `src/lib/messaging/channels/zalo/template-resolver.ts` `zaloGroupPolicy()`, and `nemoclaw-blueprint/policies/presets/zalo.yaml` rules for GET/POST `"/**"`.
  • Evidence: `botToken` only has `envKey: "ZALO_BOT_TOKEN"` and prompt text, `allowedIds` only has `envKey: "ZALO_ALLOWED_IDS"`, `zaloGroupPolicy()` returns `nonEmptyString(stateValue(context, "zaloConfig.groupPolicy")) ?? "allowlist"`, and `zalo.yaml` permits `{ method: GET, path: "/**" }` plus `{ method: POST, path: "/**" }`.

PRA-6 Resolve/justify — Zalo authorization render coverage stops before the allowlist path

  • Location: src/lib/messaging/compiler/manifest-compiler.test.ts:339
  • Category: tests
  • Problem: The compiler test pins the flat Zalo render, placeholder token, default `groupPolicy`, and omitted `dmPolicy`/`allowFrom` when no allowlist is configured, but it does not compile Zalo with `ZALO_ALLOWED_IDS` set.
  • Impact: A future resolver or compiler refactor could silently fail to enforce the operator-provided DM allowlist, fail to deduplicate IDs, or render the wrong OpenClaw shape while the current tests still pass.
  • Recommended action: Add a focused Zalo compiler case with numeric `ZALO_ALLOWED_IDS` and assert the final `channels.zalo` value contains `dmPolicy: "allowlist"` and the expected deduplicated numeric `allowFrom` array while remaining flat with no `accounts` key.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the Zalo assertions near the `zaloRender` lookup in `src/lib/messaging/compiler/manifest-compiler.test.ts`; they only cover the no-allowlist case.
  • Missing regression test: Add a test named `compiles Zalo allowed IDs as dmPolicy allowlist with deduplicated numeric allowFrom`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the Zalo assertions near the `zaloRender` lookup in `src/lib/messaging/compiler/manifest-compiler.test.ts`; they only cover the no-allowlist case.
  • Evidence: The test comment says `dmPolicy/allowFrom stay omitted without an allowlist`, and the assertions check `not.toHaveProperty("dmPolicy")` and `not.toHaveProperty("allowFrom")`; no changed test compiles Zalo with `ZALO_ALLOWED_IDS` populated.

PRA-7 Resolve/justify — `channels add zalo` lacks workflow-side provider binding evidence

  • Location: test/channels-add-preset.test.ts:316
  • Category: workflow
  • Problem: The preset-ordering loop now includes `zalo`, which proves `applyPreset("zalo")` runs before the rebuild prompt. The harness already captures `providerCalls`, but the Zalo loop only serializes and asserts `appliedCalls` and `callOrder`.
  • Impact: A regression in add-channel workflow application could leave a rebuilt sandbox with the Zalo preset and rendered channel state but without the OpenShell provider that supplies `ZALO_BOT_TOKEN` at runtime.
  • Recommended action: Extend the Zalo case in the existing add-channel harness, or add a focused Zalo test, to assert `providerCalls` contains `test-sb-zalo-bridge` with env key `ZALO_BOT_TOKEN` before the rebuild prompt.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the loop over `["telegram", "slack", "discord", "zalo"]` in `test/channels-add-preset.test.ts`; the result payload omits `providerCalls` even though the preamble records them.
  • Missing regression test: Add a test named `channels add zalo registers test-sb-zalo-bridge with ZALO_BOT_TOKEN before triggering rebuild`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the loop over `["telegram", "slack", "discord", "zalo"]` in `test/channels-add-preset.test.ts`; the result payload omits `providerCalls` even though the preamble records them.
  • Evidence: The compiler unit test asserts `demo-zalo-bridge`, but the workflow-level `channels add` regression for Zalo only checks `appliedCalls` and `callOrder`.

PRA-8 Resolve/justify — Flat `channels.zalo` compatibility needs a source boundary and removal condition

  • Location: src/lib/messaging/channels/zalo/manifest.ts:83
  • Category: architecture
  • Problem: The PR intentionally renders Zalo as a flat `channels.zalo` object because `@openclaw/zalo` rejects the shared `accounts.default` shape. The comment identifies the rejection, and tests pin the flat shape, but the changed code does not state why NemoClaw cannot use the shared shape until upstream changes or when this special case can be removed.
  • Impact: Future maintainers may preserve a plugin-specific workaround after it is no longer needed or refactor it back to the shared shape without understanding the upstream schema dependency.
  • Recommended action: Expand the manifest comment or nearby test comment to identify the upstream `@openclaw/zalo` schema as the source boundary and state the removal condition, for example that the flat render can be revisited once the plugin accepts the shared `accounts.default` schema.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the comment above `path: "channels.zalo"` in `src/lib/messaging/channels/zalo/manifest.ts` and the compiler test comment near the Zalo render assertions; neither gives a removal condition.
  • Missing regression test: Existing tests already prove the flat shape through `declares Zalo as an OpenClaw-only flat-render channel with allowlist config` and the compiler `zaloRender` assertions; no additional shape test is needed beyond documenting the boundary/removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the comment above `path: "channels.zalo"` in `src/lib/messaging/channels/zalo/manifest.ts` and the compiler test comment near the Zalo render assertions; neither gives a removal condition.
  • Evidence: The manifest comment says the plugin rejects `accounts.default` with `must not have additional properties`, and compiler tests assert no `accounts`, but neither location states when the compatibility behavior should be removed.

PRA-9 Resolve/justify — New `@openclaw/zalo` installer boundary needs a local trust contract

  • Location: src/lib/messaging/channels/zalo/manifest.ts:119
  • Category: security
  • Problem: The PR adds a required sandbox image build install of `npm:@openclaw/zalo@{{openclaw.version}}` for a credentialed messaging bridge. The spec is pinned and OpenClaw-scoped like other OpenClaw plugins, and tests cover package enumeration, but the changed code does not document what package/source/version contract NemoClaw is relying on for this new runtime code.
  • Impact: If the upstream package contract or version mapping changes unexpectedly, NemoClaw can build images that install a plugin with a different runtime schema or credential behavior and only discover the mismatch after enabling the channel.
  • Recommended action: Add a short in-code or test comment next to `agentPackages` identifying the trusted upstream package/source contract and why the `{{openclaw.version}}` pin is the intended compatibility boundary for Zalo.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect `zaloManifest.agentPackages` and the package assertions in `src/lib/messaging/channels/metadata.test.ts`; they assert the spec but do not describe the trusted source/version contract.
  • Missing regression test: Existing tests cover `npm:@openclaw/zalo@{{openclaw.version}}` for OpenClaw and no Hermes package specs; add only a comment-level contract unless maintainers want an explicit test named `documents the pinned OpenClaw Zalo plugin source contract`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `zaloManifest.agentPackages` and the package assertions in `src/lib/messaging/channels/metadata.test.ts`; they assert the spec but do not describe the trusted source/version contract.
  • Evidence: `zaloManifest.agentPackages` installs `npm:@openclaw/zalo@{{openclaw.version}}` with `pin: true` and `required: true`; no nearby comment explains the trusted upstream/version compatibility contract.

💡 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.

  • None.
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 — rejects malformed ZALO_BOT_TOKEN before saving the Zalo provider. The PR adds a credentialed OpenClaw-only messaging bridge, required sandbox plugin install, OpenClaw config render, health hook, provider binding, and network policy preset. Static tests cover registration and happy paths, but branch/negative tests and targeted runtime validation are still advisable for this sandbox/plugin/policy boundary.
  • PRA-T2 Runtime validation — rejects non-numeric ZALO_ALLOWED_IDS before rendering Zalo allowFrom. The PR adds a credentialed OpenClaw-only messaging bridge, required sandbox plugin install, OpenClaw config render, health hook, provider binding, and network policy preset. Static tests cover registration and happy paths, but branch/negative tests and targeted runtime validation are still advisable for this sandbox/plugin/policy boundary.
  • PRA-T3 Runtime validation — falls back to allowlist for invalid persisted ZALO_GROUP_POLICY. The PR adds a credentialed OpenClaw-only messaging bridge, required sandbox plugin install, OpenClaw config render, health hook, provider binding, and network policy preset. Static tests cover registration and happy paths, but branch/negative tests and targeted runtime validation are still advisable for this sandbox/plugin/policy boundary.
  • PRA-T4 Runtime validation — compiles Zalo allowed IDs as dmPolicy allowlist with deduplicated numeric allowFrom. The PR adds a credentialed OpenClaw-only messaging bridge, required sandbox plugin install, OpenClaw config render, health hook, provider binding, and network policy preset. Static tests cover registration and happy paths, but branch/negative tests and targeted runtime validation are still advisable for this sandbox/plugin/policy boundary.
  • PRA-T5 Runtime validation — channels add zalo registers test-sb-zalo-bridge with ZALO_BOT_TOKEN before triggering rebuild. The PR adds a credentialed OpenClaw-only messaging bridge, required sandbox plugin install, OpenClaw config render, health hook, provider binding, and network policy preset. Static tests cover registration and happy paths, but branch/negative tests and targeted runtime validation are still advisable for this sandbox/plugin/policy boundary.
  • PRA-T6 Zalo authorization render coverage stops before the allowlist path — Add a focused Zalo compiler case with numeric `ZALO_ALLOWED_IDS` and assert the final `channels.zalo` value contains `dmPolicy: "allowlist"` and the expected deduplicated numeric `allowFrom` array while remaining flat with no `accounts` key.
  • PRA-T7 Acceptance clause — each manifest compiles into a stable `SandboxMessagingPlan` — add test evidence or identify existing coverage. `src/lib/messaging/compiler/manifest-compiler.test.ts` includes Zalo in the deterministic OpenClaw plan, but the Zalo allowlist branch and invalid group-policy behavior are not covered.
  • PRA-T8 Acceptance clause — policy plans match current channel policy behavior — add test evidence or identify existing coverage. Zalo policy planning is wired through `policyPresets: [{ name: "zalo" }]`, compiler assertions include the `zalo` network policy entry, and `test/channels-add-preset.test.ts` asserts the preset is applied before rebuild; the preset's `/**` route scope still needs narrowing or a local justification test.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: Zalo flat `channels.zalo` OpenClaw render

  • 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: Existing manifest/compiler tests assert `path: "channels.zalo"` and no `accounts` key.
  • 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: `src/lib/messaging/channels/zalo/manifest.ts` comments on the rejection and `src/lib/messaging/compiler/manifest-compiler.test.ts` pins the flat render, but neither records a removal condition.

PRA-2 Resolve/justify — Source-of-truth review needed: Zalo group-policy render default

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • 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: No current Zalo test covers invalid persisted/direct `ZALO_GROUP_POLICY` render behavior.
  • 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: `src/lib/messaging/channels/zalo/template-resolver.ts` returns `nonEmptyString(stateValue(context, "zaloConfig.groupPolicy")) ?? "allowlist"`.

PRA-3 Resolve/justify — Source-of-truth review needed: Zalo Bot API policy wildcard

  • 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: No current policy test pins a narrowed route set or records a wildcard justification for Zalo.
  • 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/zalo.yaml` says long-polling `getUpdates` is GET and sending messages is POST, but both rules use `path: "/**"`.

PRA-4 Resolve/justify — Source-of-truth review needed: Zalo OpenClaw plugin install

  • 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: Existing tests assert the package spec; a comment-level contract is the missing evidence unless maintainers want a dedicated documentation assertion.
  • 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: `zaloManifest.agentPackages` sets `spec: "npm:@openclaw/zalo@{{openclaw.version}}"`, `pin: true`, and `required: true` without a nearby source/version contract.

PRA-5 Resolve/justify — Zalo bridge trust-boundary validation and egress policy are too broad

  • Location: src/lib/messaging/channels/zalo/manifest.ts:24
  • Category: security
  • Problem: The Zalo manifest documents `ZALO_BOT_TOKEN` as `id:secret` and `ZALO_ALLOWED_IDS` as numeric IDs, but neither input declares a `formatPattern`/`formatHint`, so the shared token/config hooks cannot enforce those contracts. The resolver also renders any non-empty `zaloConfig.groupPolicy` instead of rechecking `open|allowlist|disabled`, while the new policy preset allows Node GET and POST to every path on `bot-api.zaloplatforms.com`.
  • Impact: Malformed credentials or non-numeric authorization entries can be saved and rendered into OpenClaw, corrupt direct/persisted group-policy state can reach the plugin, and the sandbox receives broader Bot API egress than the documented long-polling/send-message flow may require.
  • Recommended action: Add manifest `formatPattern`/`formatHint` for `ZALO_BOT_TOKEN` and comma-separated numeric `ZALO_ALLOWED_IDS`, mirror Telegram's render-time group-policy allowlist with a safe `allowlist` fallback or explicit refusal, and narrow `zalo.yaml` to the required Bot API routes or add an in-repo policy comment/test proving why `/**` is required.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/messaging/channels/zalo/manifest.ts` inputs for absent `formatPattern`, `src/lib/messaging/channels/zalo/template-resolver.ts` `zaloGroupPolicy()`, and `nemoclaw-blueprint/policies/presets/zalo.yaml` rules for GET/POST `"/**"`.
  • Missing regression test: Add tests named `rejects malformed ZALO_BOT_TOKEN before saving the Zalo provider`, `rejects non-numeric ZALO_ALLOWED_IDS before rendering Zalo allowFrom`, `falls back to allowlist for invalid persisted ZALO_GROUP_POLICY`, and `zalo policy preset limits runtime access to documented Bot API routes or records the wildcard justification`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/messaging/channels/zalo/manifest.ts` inputs for absent `formatPattern`, `src/lib/messaging/channels/zalo/template-resolver.ts` `zaloGroupPolicy()`, and `nemoclaw-blueprint/policies/presets/zalo.yaml` rules for GET/POST `"/**"`.
  • Evidence: `botToken` only has `envKey: "ZALO_BOT_TOKEN"` and prompt text, `allowedIds` only has `envKey: "ZALO_ALLOWED_IDS"`, `zaloGroupPolicy()` returns `nonEmptyString(stateValue(context, "zaloConfig.groupPolicy")) ?? "allowlist"`, and `zalo.yaml` permits `{ method: GET, path: "/**" }` plus `{ method: POST, path: "/**" }`.

PRA-6 Resolve/justify — Zalo authorization render coverage stops before the allowlist path

  • Location: src/lib/messaging/compiler/manifest-compiler.test.ts:339
  • Category: tests
  • Problem: The compiler test pins the flat Zalo render, placeholder token, default `groupPolicy`, and omitted `dmPolicy`/`allowFrom` when no allowlist is configured, but it does not compile Zalo with `ZALO_ALLOWED_IDS` set.
  • Impact: A future resolver or compiler refactor could silently fail to enforce the operator-provided DM allowlist, fail to deduplicate IDs, or render the wrong OpenClaw shape while the current tests still pass.
  • Recommended action: Add a focused Zalo compiler case with numeric `ZALO_ALLOWED_IDS` and assert the final `channels.zalo` value contains `dmPolicy: "allowlist"` and the expected deduplicated numeric `allowFrom` array while remaining flat with no `accounts` key.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the Zalo assertions near the `zaloRender` lookup in `src/lib/messaging/compiler/manifest-compiler.test.ts`; they only cover the no-allowlist case.
  • Missing regression test: Add a test named `compiles Zalo allowed IDs as dmPolicy allowlist with deduplicated numeric allowFrom`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the Zalo assertions near the `zaloRender` lookup in `src/lib/messaging/compiler/manifest-compiler.test.ts`; they only cover the no-allowlist case.
  • Evidence: The test comment says `dmPolicy/allowFrom stay omitted without an allowlist`, and the assertions check `not.toHaveProperty("dmPolicy")` and `not.toHaveProperty("allowFrom")`; no changed test compiles Zalo with `ZALO_ALLOWED_IDS` populated.

PRA-7 Resolve/justify — `channels add zalo` lacks workflow-side provider binding evidence

  • Location: test/channels-add-preset.test.ts:316
  • Category: workflow
  • Problem: The preset-ordering loop now includes `zalo`, which proves `applyPreset("zalo")` runs before the rebuild prompt. The harness already captures `providerCalls`, but the Zalo loop only serializes and asserts `appliedCalls` and `callOrder`.
  • Impact: A regression in add-channel workflow application could leave a rebuilt sandbox with the Zalo preset and rendered channel state but without the OpenShell provider that supplies `ZALO_BOT_TOKEN` at runtime.
  • Recommended action: Extend the Zalo case in the existing add-channel harness, or add a focused Zalo test, to assert `providerCalls` contains `test-sb-zalo-bridge` with env key `ZALO_BOT_TOKEN` before the rebuild prompt.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the loop over `["telegram", "slack", "discord", "zalo"]` in `test/channels-add-preset.test.ts`; the result payload omits `providerCalls` even though the preamble records them.
  • Missing regression test: Add a test named `channels add zalo registers test-sb-zalo-bridge with ZALO_BOT_TOKEN before triggering rebuild`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the loop over `["telegram", "slack", "discord", "zalo"]` in `test/channels-add-preset.test.ts`; the result payload omits `providerCalls` even though the preamble records them.
  • Evidence: The compiler unit test asserts `demo-zalo-bridge`, but the workflow-level `channels add` regression for Zalo only checks `appliedCalls` and `callOrder`.

PRA-8 Resolve/justify — Flat `channels.zalo` compatibility needs a source boundary and removal condition

  • Location: src/lib/messaging/channels/zalo/manifest.ts:83
  • Category: architecture
  • Problem: The PR intentionally renders Zalo as a flat `channels.zalo` object because `@openclaw/zalo` rejects the shared `accounts.default` shape. The comment identifies the rejection, and tests pin the flat shape, but the changed code does not state why NemoClaw cannot use the shared shape until upstream changes or when this special case can be removed.
  • Impact: Future maintainers may preserve a plugin-specific workaround after it is no longer needed or refactor it back to the shared shape without understanding the upstream schema dependency.
  • Recommended action: Expand the manifest comment or nearby test comment to identify the upstream `@openclaw/zalo` schema as the source boundary and state the removal condition, for example that the flat render can be revisited once the plugin accepts the shared `accounts.default` schema.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the comment above `path: "channels.zalo"` in `src/lib/messaging/channels/zalo/manifest.ts` and the compiler test comment near the Zalo render assertions; neither gives a removal condition.
  • Missing regression test: Existing tests already prove the flat shape through `declares Zalo as an OpenClaw-only flat-render channel with allowlist config` and the compiler `zaloRender` assertions; no additional shape test is needed beyond documenting the boundary/removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the comment above `path: "channels.zalo"` in `src/lib/messaging/channels/zalo/manifest.ts` and the compiler test comment near the Zalo render assertions; neither gives a removal condition.
  • Evidence: The manifest comment says the plugin rejects `accounts.default` with `must not have additional properties`, and compiler tests assert no `accounts`, but neither location states when the compatibility behavior should be removed.

PRA-9 Resolve/justify — New `@openclaw/zalo` installer boundary needs a local trust contract

  • Location: src/lib/messaging/channels/zalo/manifest.ts:119
  • Category: security
  • Problem: The PR adds a required sandbox image build install of `npm:@openclaw/zalo@{{openclaw.version}}` for a credentialed messaging bridge. The spec is pinned and OpenClaw-scoped like other OpenClaw plugins, and tests cover package enumeration, but the changed code does not document what package/source/version contract NemoClaw is relying on for this new runtime code.
  • Impact: If the upstream package contract or version mapping changes unexpectedly, NemoClaw can build images that install a plugin with a different runtime schema or credential behavior and only discover the mismatch after enabling the channel.
  • Recommended action: Add a short in-code or test comment next to `agentPackages` identifying the trusted upstream package/source contract and why the `{{openclaw.version}}` pin is the intended compatibility boundary for Zalo.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect `zaloManifest.agentPackages` and the package assertions in `src/lib/messaging/channels/metadata.test.ts`; they assert the spec but do not describe the trusted source/version contract.
  • Missing regression test: Existing tests cover `npm:@openclaw/zalo@{{openclaw.version}}` for OpenClaw and no Hermes package specs; add only a comment-level contract unless maintainers want an explicit test named `documents the pinned OpenClaw Zalo plugin source contract`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `zaloManifest.agentPackages` and the package assertions in `src/lib/messaging/channels/metadata.test.ts`; they assert the spec but do not describe the trusted source/version contract.
  • Evidence: `zaloManifest.agentPackages` installs `npm:@openclaw/zalo@{{openclaw.version}}` with `pin: true` and `required: true`; no nearby comment explains the trusted upstream/version compatibility contract.

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.

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: messaging-providers-vitest, channels-add-remove-vitest, network-policy-vitest
Optional E2E: channels-stop-start-openclaw-e2e, diagnostics-e2e

Dispatch hint: messaging-providers-vitest,channels-add-remove-vitest,network-policy-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 adds a token-backed messaging channel/provider and OpenClaw config rendering. This live scenario validates provider creation, OpenShell placeholder preservation, credential isolation, policy attachment, runtime config, and messaging bridge behavior across a real sandbox boundary.
  • channels-add-remove-vitest (high): Required because Zalo is added to supported OpenClaw channels and channel preset application was changed in tests. This scenario validates the user-facing channels add/rebuild/remove lifecycle, registry messaging plan persistence, gateway credential reuse, and policy application before rebuild.
  • network-policy-vitest (high): Required because the PR adds a new enforced network policy preset. This live scenario exercises restricted sandbox egress, policy-add behavior, and allow/deny enforcement through OpenShell, providing merge-blocking confidence for the network security boundary touched by the Zalo preset.

Optional E2E

  • channels-stop-start-openclaw-e2e (high): Optional adjacent confidence for the new OpenClaw-only channel metadata: validates stop/start/rebuild persistence for messaging channels and provider records, but the PR does not directly modify stop/start implementation.
  • diagnostics-e2e (medium): Optional because diagnostics metadata now includes Zalo. Useful to confirm user-facing channel diagnostics still work against a live sandbox, but unit tests cover most static diagnostics derivation.

New E2E recommendations

  • Zalo messaging provider end-to-end coverage (high): Existing messaging provider E2E coverage is oriented around Telegram, Discord, Slack, WeChat, and WhatsApp and does not appear to assert Zalo-specific provider names, ZALO_BOT_TOKEN placeholder resolution, flat channels.zalo rendering, plugins.entries.zalo, group policy/default allowlist behavior, or raw-token absence in sandbox-visible surfaces.
    • Suggested test: Add a Zalo-focused live messaging scenario using fake ZALO_BOT_TOKEN that onboards/adds Zalo, asserts demo-zalo-bridge provider wiring, openshell:resolve:env:ZALO_BOT_TOKEN in openclaw.json, no raw token leakage, Zalo plugin package installation metadata, and the flat channels.zalo shape without accounts.default.
  • Zalo network policy validation (high): The new Zalo preset allows GET and POST to bot-api.zaloplatforms.com, but existing live network policy tests do not specifically prove this preset's endpoint, allowed methods, or denied off-preset Zalo-like hosts.
    • Suggested test: Extend the network-policy or messaging-providers E2E with a Zalo preset probe that applies zalo, confirms the gateway policy contains bot-api.zaloplatforms.com, verifies Node HTTPS GET/POST are permitted through the sandbox policy path, and verifies a neighboring unauthorized host/method is denied.

Dispatch hint

  • Workflow: e2e-vitest-scenarios.yaml
  • jobs input: messaging-providers-vitest,channels-add-remove-vitest,network-policy-vitest

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: The PR adds Zalo to the OpenClaw agent manifest and built-in messaging/channel policy plumbing. Trusted main does not currently expose a live-supported Zalo-specific typed scenario, so run the smallest live-supported OpenClaw registry scenario to validate repo-current OpenClaw onboarding still works after the manifest and messaging registry changes.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • agents/openclaw/manifest.yaml
  • nemoclaw-blueprint/policies/presets/zalo.yaml
  • src/lib/messaging-channel-config.test.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/zalo/hooks/index.ts
  • src/lib/messaging/channels/zalo/hooks/openclaw-bridge-health.ts
  • src/lib/messaging/channels/zalo/manifest.ts
  • src/lib/messaging/channels/zalo/template-resolver.ts
  • src/lib/messaging/hooks/builtins.ts
  • src/lib/onboard/messaging-prep.test.ts
  • src/lib/sandbox/channels.test.ts

@hunglp6d hunglp6d self-assigned this Jun 22, 2026
@hunglp6d hunglp6d added VRDC Issues and PRs submitted by NVIDIA VRDC test team. area: docs Documentation, examples, guides, or docs build area: messaging Messaging channels, bridges, manifests, or channel lifecycle area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow feature PR adds or expands user-visible functionality labels Jun 22, 2026
@hunglp6d hunglp6d marked this pull request as ready for review June 22, 2026 03:18

@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.

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@docs/manage-sandboxes/messaging-channels.mdx`:
- Line 7: Update the description-agent field on line 7 to include Zalo in the
list of supported messaging channels. Currently the field mentions Telegram,
Discord, Slack, WeChat, and WhatsApp. Add Zalo to this list to match the
channels that are actually documented on the page.
- Line 142: The host-side command example in the messaging-channels.mdx file
uses `nemoclaw` but should use `$$nemoclaw` to align with coding guidelines for
shared OpenClaw and Hermes documentation pages. Replace `nemoclaw <sandbox> exec
-- openclaw pairing approve zalo <code>` with `$$nemoclaw <sandbox> exec --
openclaw pairing approve zalo <code>` to ensure consistency across the shared
documentation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b44592fa-3db4-4653-bb48-6eee5b83e362

📥 Commits

Reviewing files that changed from the base of the PR and between cf403cf and 798a3af.

📒 Files selected for processing (25)
  • agents/openclaw/manifest.yaml
  • docs/manage-sandboxes/messaging-channels.mdx
  • docs/network-policy/integration-policy-examples.mdx
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • nemoclaw-blueprint/policies/presets/zalo.yaml
  • src/lib/agent/defs.test.ts
  • src/lib/messaging-channel-config.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/metadata.ts
  • src/lib/messaging/channels/template-resolver.ts
  • src/lib/messaging/channels/zalo/hooks/index.ts
  • src/lib/messaging/channels/zalo/hooks/openclaw-bridge-health.ts
  • src/lib/messaging/channels/zalo/manifest.ts
  • src/lib/messaging/channels/zalo/template-resolver.ts
  • src/lib/messaging/diagnostics.test.ts
  • src/lib/messaging/hooks/builtins.ts
  • src/lib/messaging/hooks/hook-runner.test.ts
  • src/lib/onboard/messaging-prep.test.ts
  • src/lib/sandbox/channels.test.ts
  • test/channels-add-preset.test.ts
  • test/policies.test.ts
  • test/sandbox-provider-cleanup.test.ts

Comment thread docs/manage-sandboxes/messaging-channels.mdx
Comment thread docs/manage-sandboxes/messaging-channels.mdx Outdated
@hunglp6d

Copy link
Copy Markdown
Contributor Author

PR Review Advisor findings:

  • 1 & 2 — Zalo covered in the compiler aggregate test + a per-channel manifests.test.ts case (flat channels.zalo, no accounts, ZALO_BOT_TOKEN placeholder, default groupPolicy, dmPolicy/allowFrom omitted). ✅
  • 5 — channels add zalo added to the preset-ordering loop (applies the zalo preset before rebuild). ✅
  • 6 — pinned npm:@openclaw/zalo@{{openclaw.version}} install spec asserted in metadata.test.ts. ✅
  • 4 — no longer applies: this PR carries no docs changes, so nothing advertises Zalo for Hermes. Zalo channel docs are deferred to a tech writer (separate follow-up).

@hunglp6d hunglp6d requested review from cv and ericksoa June 22, 2026 08:18
@hunglp6d hunglp6d requested a review from sandl99 June 22, 2026 08:18
@sandl99 sandl99 added the v0.0.66 Release target label Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build area: messaging Messaging channels, bridges, manifests, or channel lifecycle area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow feature PR adds or expands user-visible functionality v0.0.66 Release target VRDC Issues and PRs submitted by NVIDIA VRDC test team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants