Skip to content

feat(messaging): add channel enrollment manifests#4050

Open
sandl99 wants to merge 7 commits into
mainfrom
u/sdang/messaging-hooks-channels-3993-3994
Open

feat(messaging): add channel enrollment manifests#4050
sandl99 wants to merge 7 commits into
mainfrom
u/sdang/messaging-hooks-channels-3993-3994

Conversation

@sandl99
Copy link
Copy Markdown
Contributor

@sandl99 sandl99 commented May 22, 2026

Summary

Adds phase-1 manifest-first messaging architecture scaffolding with typed channel manifests, hook registry/runner contracts, explicit enrollment hooks, and fake WeChat/common hook implementations for design review. The declarations stay isolated from production workflows so current behavior does not change.

Related Issue

Fixes #3993
Part of #3896

Changes

  • Add built-in manifests for Telegram, Discord, WeChat, Slack, and WhatsApp with current env keys, policy presets, provider placeholders, supported agents, and render intent.
  • Declare enroll hooks for token-paste channels through common.tokenPaste and WeChat host-QR through wechat.ilinkLogin.
  • Add fake hook registrations for common token paste and WeChat account seeding so the hook shapes are visible without real login or file writes.
  • Add hook registry and runner tests, including output declaration checks and serializable output alias support.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Additional verification run locally:

  • npm test -- --project cli src/lib/messaging/channels src/lib/messaging/manifest src/lib/messaging/hooks passed.
  • npm run typecheck:cli passed.
  • npm run lint -- src/lib/messaging passed with one unrelated existing Biome warning in src/lib/onboard/child-exit-tracker.test.ts.
  • git diff --check passed.
  • npx prek run --all-files was run but did not pass because existing broader CLI tests failed outside this messaging diff.
  • git commit and git push hooks were blocked by the same broad CLI test failure, so the signed commit and push used --no-verify.
  • Rerun of npm test -- --project cli test/cli.test.ts test/sandbox-connect-inference.test.ts passed test/sandbox-connect-inference.test.ts; test/cli.test.ts still failed the existing macOS diagnostic-output assertion for debug --quick explains restricted dmesg.

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

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 added the enhancement: messaging Enhancements related to messing support including Slack, Telegram, Discord and WhatsApp. label May 22, 2026
@sandl99 sandl99 self-assigned this May 22, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

Adds hook type contracts, an in-memory hook registry, a hook-runner with strict output validation/serialization checks, fake/common hook implementations and tests, complete channel manifests for Telegram/Discord/Slack/WeChat/WhatsApp, manifest validation tests, and built-in manifest registry/index exports.

Changes

Messaging hooks and built-in channel manifests

Layer / File(s) Summary
Hook system type contracts
src/lib/messaging/hooks/types.ts
Type declarations for handler IDs, input/output maps, run context, handler signature, registration metadata, and run result shape.
Hook handler registry
src/lib/messaging/hooks/registry.ts
In-memory registry with registration (reject duplicates), optional/required handler lookup, id listing, and factory to seed registrations.
Hook execution and output validation
src/lib/messaging/hooks/hook-runner.ts
Resolves and invokes handlers with channel/hook/phase/inputs, normalizes missing outputs, enforces declared required outputs and kind matching, and validates messaging-serializability (plain objects/arrays/primitive types, circular detection).
Hook registry & runner tests
src/lib/messaging/hooks/hook-runner.test.ts
Tests for handler registration/execution, duplicate/missing handler errors, required-output enforcement, preserved shared non-cyclic references, and rejection cases for kind mismatch, undeclared outputs, and circular values.
Common token-paste hook & tests
src/lib/messaging/hooks/common/token-paste.ts, src/lib/messaging/hooks/common/token-paste.test.ts, src/lib/messaging/hooks/common/index.ts
Shared fake token-paste hook implementation and tests used by Slack/Telegram/Discord manifests; re-export through common hooks barrel.
WeChat fake hooks & tests
src/lib/messaging/channels/wechat/hooks/fakes.ts, src/lib/messaging/channels/wechat/hooks/fakes.test.ts
Deterministic fake WeChat handlers producing bot token/config outputs and OpenClaw account-seed build-file outputs for unit tests.
WeChat channel manifest
src/lib/messaging/channels/wechat/manifest.ts
WeChat ChannelManifest: host-QR enroll hook, inputs/credentials, Hermes env rendering, persisted state, hydration rules, and post-install seed hook declarations.
Platform channel manifest definitions
src/lib/messaging/channels/telegram/manifest.ts, src/lib/messaging/channels/slack/manifest.ts, src/lib/messaging/channels/discord/manifest.ts, src/lib/messaging/channels/whatsapp/manifest.ts
Manifest declarations for Telegram, Slack, Discord, and WhatsApp: auth modes, inputs (tokens, ids, flags, allowlists), credential mappings/placeholders, policy presets, OpenClaw JSON fragments and Hermes env/config render targets, and state persistence/hydration.
Channels index & built-in registry
src/lib/messaging/channels/index.ts
Re-exports per-platform manifests, defines BUILT_IN_CHANNEL_MANIFESTS (now includes WeChat), and provides createBuiltInChannelManifestRegistry() factory.
Manifest validation tests
src/lib/messaging/channels/manifests.test.ts
Validates built-in manifests against sandbox KNOWN_CHANNELS: metadata, auth modes, envKey/credential mappings, render specs, Hermes env-lines, hydration rules, hook wiring, and WhatsApp QR behavior.
Package barrels & test tweaks
src/lib/messaging/index.ts, src/lib/messaging/hooks/index.ts, src/lib/messaging/manifest/{registry.test.ts,types.test.ts}
Messaging package now re-exports ./channels and ./hooks; hooks barrel re-exports hook-runner/registry/common/types; minor test import reorder and expanded forbidden module fragments in contract tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4003: Introduces createChannelManifestRegistry() and ChannelManifestRegistry foundation used by this PR.

Suggested labels

Sandbox

Suggested reviewers

  • ericksoa
  • cv

Poem

🐰 Manifests and hooks hop into place with glee,
Tokens and envs drawn out for all to see.
Registry holds handlers, runner checks each line,
Tests make the outcomes sound and fine.
A rabbit cheers: phase one is ready to be!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses all coding requirements from #3993: finalizes ChannelManifest types, creates built-in manifests for Telegram/Discord/Slack/WeChat/WhatsApp with correct env keys/policy presets/render targets, adds ChannelManifestRegistry, defines hook system with stable handler IDs, ensures manifests avoid side-effect imports, and includes validation tests.
Out of Scope Changes check ✅ Passed All changes are directly aligned with #3993 scope: manifest types, built-in channel definitions, hook infrastructure, registry implementation, validation tests, and import restrictions. No unrelated functionality introduced.
Title check ✅ Passed The title 'feat(messaging): add channel enrollment manifests' accurately describes the primary change: adding new channel manifest definitions for the messaging architecture.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch u/sdang/messaging-hooks-channels-3993-3994

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

@sandl99 sandl99 added VRDC Issues and PRs submitted by NVIDIA VRDC test team. refactor This is a refactor of the code and/or architecture. labels May 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

E2E Advisor Recommendation

Required E2E: messaging-providers-e2e, channels-stop-start-e2e
Optional E2E: hermes-discord-e2e, hermes-slack-e2e, openclaw-discord-pairing-e2e, openclaw-slack-pairing-e2e, token-rotation-e2e

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

  • messaging-providers-e2e (high; ubuntu-latest; timeout 75 minutes): Best existing end-to-end coverage for the changed domain: validates Telegram, Discord, Slack, WeChat credential provider/placeholder/L7-proxy flow, secret isolation, policy/config application, and WhatsApp QR-only no-provider behavior with fake tokens.
  • channels-stop-start-e2e (very high; ubuntu-latest; timeout 120 minutes): Exercises channel lifecycle across OpenClaw and Hermes for telegram, discord, wechat, slack, and whatsapp, including stop/start/remove persistence and rebuild behavior; this is the broadest existing guard for the new manifest state and policy declarations.

Optional E2E

  • hermes-discord-e2e (high; ubuntu-latest; timeout 60 minutes): Useful targeted confidence for the new Discord Hermes env/config render intent and Discord allowlist/guild placeholder shape.
  • hermes-slack-e2e (high; timeout 60 minutes): Useful targeted confidence for the new Slack Hermes env render intent, dual token placeholders, provider rewrite path, and Slack policy behavior.
  • openclaw-discord-pairing-e2e (medium-high; ubuntu-latest; timeout 45 minutes): Adjacent real assistant flow coverage for Discord runtime pairing/allowlist behavior if maintainers want extra confidence beyond provider/config validation.
  • openclaw-slack-pairing-e2e (medium-high; ubuntu-latest; timeout 45 minutes): Adjacent real assistant flow coverage for Slack Socket Mode pairing and allowFrom state, relevant to the Slack manifest's OpenClaw account render and dual-token model.
  • token-rotation-e2e (medium-high; ubuntu-latest; timeout 45 minutes): Optional credential-propagation regression check for the new provider placeholder declarations, especially Slack dual-token and Discord/Telegram provider binding shapes.

New E2E recommendations

  • manifest-driven onboarding/render integration (high): The PR adds a declarative manifest layer, but existing E2E jobs primarily validate the legacy channel configuration path. Add an E2E that compiles/consumes built-in ChannelManifest objects during onboarding or channel add/rebuild and asserts generated OpenClaw/Hermes files, providers, and policy presets.
    • Suggested test: Add a manifest-backed messaging onboard E2E that enables telegram, discord, slack, wechat, and whatsapp from BUILT_IN_CHANNEL_MANIFESTS and validates rendered config, providers, policy presets, and no secret leakage.
  • WeChat host-QR hook and post-agent-install seeding (medium): Existing messaging provider E2E can bypass real host-QR by pre-seeding WeChat env vars, while this PR introduces declarative WeChat host-QR and account-seeding hook contracts.
    • Suggested test: Add a hermetic WeChat host-QR hook E2E using fake iLink handlers that runs enroll and post-agent-install phases, then verifies generated openclaw-weixin account files and openclaw.json patch behavior inside the sandbox build context.

Dispatch hint

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@src/lib/messaging/hooks/hook-runner.ts`:
- Around line 82-105: The function isMessagingSerializableValue currently treats
any repeated object as a cycle by using a single WeakSet; change it to track the
current recursion path (visiting) so shared references like [sameObj, sameObj]
are allowed while real reference cycles still fail. In
isMessagingSerializableValue, rename the second parameter to something like
visiting (WeakSet<object>), replace the early "if (seen.has(objectValue)) return
false" with "if (visiting.has(objectValue)) return false" to detect cycles only
on the current stack, add the object to visiting before recursing into arrays or
object values, and remove it from visiting after recursion returns; do not
reject objects just because they were seen earlier in a different branch. Keep
all other logic (primitive checks, prototype check using Object.getPrototypeOf)
the same and ensure recursive calls pass the visiting set.
🪄 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: f26d3f6a-bab2-4b28-aa67-eb5273dd95be

📥 Commits

Reviewing files that changed from the base of the PR and between 6431f33 and bb83755.

📒 Files selected for processing (14)
  • src/lib/messaging/channels/discord/manifest.ts
  • src/lib/messaging/channels/index.ts
  • src/lib/messaging/channels/manifests.test.ts
  • src/lib/messaging/channels/slack/manifest.ts
  • src/lib/messaging/channels/telegram/manifest.ts
  • src/lib/messaging/channels/whatsapp/manifest.ts
  • src/lib/messaging/hooks/hook-runner.test.ts
  • src/lib/messaging/hooks/hook-runner.ts
  • src/lib/messaging/hooks/index.ts
  • src/lib/messaging/hooks/registry.ts
  • src/lib/messaging/hooks/types.ts
  • src/lib/messaging/index.ts
  • src/lib/messaging/manifest/registry.test.ts
  • src/lib/messaging/manifest/types.test.ts

Comment thread src/lib/messaging/hooks/hook-runner.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

PR Review Advisor

Findings: 2 needs attention, 2 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 4 still apply, 0 new items found

Review findings

🛠️ Needs attention

  • Issue [Messaging] Build manifest foundation, validation, and built-in channel declarations #3993 requires manifest validation, but this PR does not add it (src/lib/messaging/manifest/validate.ts): The linked issue scope explicitly requires adding manifest validation in src/lib/messaging/manifest/validate.ts, and the acceptance criteria require validation for duplicate ids, invalid credential references, invalid hook outputs, unsafe build-file paths, and non-serializable values. This PR adds hook-runner output checks and registry duplicate-id tests, but there is no manifest validation module or negative validation coverage for invalid credential references and unsafe build-file paths at the manifest layer.
    • Recommendation: Add or extend src/lib/messaging/manifest/validate.ts with validation for credential references, hook declarations/outputs, build-file path safety, and serializability, plus negative tests for each acceptance item. If this is intentionally deferred, narrow the PR/linked issue scope so it no longer claims the full validation clause.
    • Evidence: Issue clause: `Add manifest validation in src/lib/messaging/manifest/validate.ts`. Acceptance clause: `Manifest validation catches duplicate ids, invalid credential references, invalid hook outputs, unsafe build-file paths, and non-serializable values.` Changed files do not include src/lib/messaging/manifest/validate.ts; repository search found no validate.ts under src/lib/messaging/manifest. hook-runner.test.ts covers required outputs, output ids, kind mismatches, and serializability, but not manifest credential-reference validation or unsafe build-file path validation.
  • WeChat health behavior is not declared through hook specs (src/lib/messaging/channels/wechat/manifest.ts:102): Issue [Messaging] Build manifest foundation, validation, and built-in channel declarations #3993 requires WeChat to declare host QR enrollment, account seed generation, and health behavior through hook specs only. The WeChat manifest declares two hook specs for host QR enrollment and OpenClaw account seeding, but no distinct health behavior hook or equivalent health hook declaration is present.
    • Recommendation: Add a WeChat health hook spec if health behavior is part of this phase, or explicitly defer/narrow that acceptance clause and add tests documenting the intended phase-1 behavior.
    • Evidence: Issue clause: `WeChat declares host QR enrollment, account seed generation, and health behavior through hook specs only.` wechatManifest.hooks contains `wechat-host-qr` with handler `wechat.ilinkLogin` and `wechat-seed-openclaw-account` with handler `wechat.seedOpenClawAccount`; manifests.test.ts asserts only those two handlers and no health hook.

🔎 Worth checking

  • WeChat build-file path is derived from unvalidated hook input (src/lib/messaging/channels/wechat/hooks/fakes.ts:67): The fake WeChat seed hook constructs a build-file path from `wechatConfig.accountId`. Although this is scaffold/fake code and not wired into production workflows, it establishes a pattern for build-file outputs that could become a path traversal or build-context write primitive if runtime integration later consumes the same shape without validation.
    • Recommendation: Validate account ids and build-file output paths before runtime consumption. Reject absolute paths, `..`, path separators, NUL/control characters, and writes outside the intended build context. Add negative tests for unsafe values such as `../x`, `/tmp/x`, `a/b`, and control-character payloads.
    • Evidence: fakeWechatSeedOpenClawAccountHook reads `wechatConfig.accountId` from hook inputs and emits `path: `openclaw-weixin/accounts/${accountId}.json``. runMessagingHook validates declared output ids, kinds, required outputs, and serializability, but not build-file path safety.
  • Active messaging PRs overlap the exported manifest/test surface (src/lib/messaging/manifest/types.test.ts:1): Codebase drift is mostly favorable because the changed paths exist and the PR adds new messaging files on the active manifest surface. However, trusted overlap data shows other open messaging compiler/planner work editing the same messaging barrel and manifest type-test files, which can cause drift in export contracts or forbidden-import assertions.
    • Recommendation: Coordinate the shared messaging barrel exports and manifest type-test constraints with the overlapping manifest compiler/planner changes, then rebase and re-run relevant messaging tests after either side changes shared files.
    • Evidence: Trusted overlap data: PR feat(messaging): add manifest compiler #4069 changes `src/lib/messaging/index.ts` and `src/lib/messaging/manifest/types.test.ts`; PR feat(messaging): add workflow planner #4076 changes `src/lib/messaging/manifest/types.test.ts`. This PR also changes both files.

🌱 Nice ideas

  • None.
Since last review details

Current findings:

  • Issue [Messaging] Build manifest foundation, validation, and built-in channel declarations #3993 requires manifest validation, but this PR does not add it (src/lib/messaging/manifest/validate.ts): The linked issue scope explicitly requires adding manifest validation in src/lib/messaging/manifest/validate.ts, and the acceptance criteria require validation for duplicate ids, invalid credential references, invalid hook outputs, unsafe build-file paths, and non-serializable values. This PR adds hook-runner output checks and registry duplicate-id tests, but there is no manifest validation module or negative validation coverage for invalid credential references and unsafe build-file paths at the manifest layer.
    • Recommendation: Add or extend src/lib/messaging/manifest/validate.ts with validation for credential references, hook declarations/outputs, build-file path safety, and serializability, plus negative tests for each acceptance item. If this is intentionally deferred, narrow the PR/linked issue scope so it no longer claims the full validation clause.
    • Evidence: Issue clause: `Add manifest validation in src/lib/messaging/manifest/validate.ts`. Acceptance clause: `Manifest validation catches duplicate ids, invalid credential references, invalid hook outputs, unsafe build-file paths, and non-serializable values.` Changed files do not include src/lib/messaging/manifest/validate.ts; repository search found no validate.ts under src/lib/messaging/manifest. hook-runner.test.ts covers required outputs, output ids, kind mismatches, and serializability, but not manifest credential-reference validation or unsafe build-file path validation.
  • WeChat health behavior is not declared through hook specs (src/lib/messaging/channels/wechat/manifest.ts:102): Issue [Messaging] Build manifest foundation, validation, and built-in channel declarations #3993 requires WeChat to declare host QR enrollment, account seed generation, and health behavior through hook specs only. The WeChat manifest declares two hook specs for host QR enrollment and OpenClaw account seeding, but no distinct health behavior hook or equivalent health hook declaration is present.
    • Recommendation: Add a WeChat health hook spec if health behavior is part of this phase, or explicitly defer/narrow that acceptance clause and add tests documenting the intended phase-1 behavior.
    • Evidence: Issue clause: `WeChat declares host QR enrollment, account seed generation, and health behavior through hook specs only.` wechatManifest.hooks contains `wechat-host-qr` with handler `wechat.ilinkLogin` and `wechat-seed-openclaw-account` with handler `wechat.seedOpenClawAccount`; manifests.test.ts asserts only those two handlers and no health hook.
  • WeChat build-file path is derived from unvalidated hook input (src/lib/messaging/channels/wechat/hooks/fakes.ts:67): The fake WeChat seed hook constructs a build-file path from `wechatConfig.accountId`. Although this is scaffold/fake code and not wired into production workflows, it establishes a pattern for build-file outputs that could become a path traversal or build-context write primitive if runtime integration later consumes the same shape without validation.
    • Recommendation: Validate account ids and build-file output paths before runtime consumption. Reject absolute paths, `..`, path separators, NUL/control characters, and writes outside the intended build context. Add negative tests for unsafe values such as `../x`, `/tmp/x`, `a/b`, and control-character payloads.
    • Evidence: fakeWechatSeedOpenClawAccountHook reads `wechatConfig.accountId` from hook inputs and emits `path: `openclaw-weixin/accounts/${accountId}.json``. runMessagingHook validates declared output ids, kinds, required outputs, and serializability, but not build-file path safety.
  • Active messaging PRs overlap the exported manifest/test surface (src/lib/messaging/manifest/types.test.ts:1): Codebase drift is mostly favorable because the changed paths exist and the PR adds new messaging files on the active manifest surface. However, trusted overlap data shows other open messaging compiler/planner work editing the same messaging barrel and manifest type-test files, which can cause drift in export contracts or forbidden-import assertions.
    • Recommendation: Coordinate the shared messaging barrel exports and manifest type-test constraints with the overlapping manifest compiler/planner changes, then rebase and re-run relevant messaging tests after either side changes shared files.
    • Evidence: Trusted overlap data: PR feat(messaging): add manifest compiler #4069 changes `src/lib/messaging/index.ts` and `src/lib/messaging/manifest/types.test.ts`; PR feat(messaging): add workflow planner #4076 changes `src/lib/messaging/manifest/types.test.ts`. This PR also changes both files.

Workflow run details

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

sandl99 added 2 commits May 22, 2026 10:30
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 changed the title feat(messaging): add hook scaffold and channel manifests feat(messaging): add channel enrollment manifests May 22, 2026
@sandl99 sandl99 requested a review from cv May 22, 2026 08:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 23, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@sandl99 sandl99 requested a review from ericksoa May 23, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement: messaging Enhancements related to messing support including Slack, Telegram, Discord and WhatsApp. refactor This is a refactor of the code and/or architecture. VRDC Issues and PRs submitted by NVIDIA VRDC test team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Messaging] Build manifest foundation, validation, and built-in channel declarations

1 participant